Skip to content

THREESCALE-10842 Preflights checks#948

Merged
MStokluska merged 12 commits into3scale:masterfrom
MStokluska:THREESCALE-10842
Mar 28, 2024
Merged

THREESCALE-10842 Preflights checks#948
MStokluska merged 12 commits into3scale:masterfrom
MStokluska:THREESCALE-10842

Conversation

@MStokluska
Copy link
Copy Markdown
Contributor

@MStokluska MStokluska commented Mar 15, 2024

Jira: https://issues.redhat.com/browse/THREESCALE-10842

What it does
The PR proposes the implementation of preflight checks for the 3scale Operator.

The goal of preflight checks is to:

  • Prevent fresh installations if external databases are used and their versions do not match versions required by 3scale

  • Prevent forced upgrades (forced upgrades being when an existing operator is uninstalled and a new version [next minor version] is applied) when database versions required are not present

  • Prevent possibility of having OLM progressing to a version that the requirements of database versions have not been met.

For example, Running 2.14 with Redis 6. 2.15 requires Redis 6.2. I've approved 2.15. It will get installed but won't reconcile [3scale instance will be unaffected and still using 2.14] until I fix the Redis version.

Another example, if on 2.15 and it requires Postgres 10, when I approve they upgrade to 2.16 (which requires Postgres 13) the upgrade won't proceed until I bump the version of Postgres. However, the difference this time is that the operator will keep reconciling 2.15 APIM changes as usual, BUT, only proceed with the upgrade once 2.16 requirements are met

  • Prevent multi-minor version hops - the operator will not reconcile APIM if it discovers that it is in multiminor version hop and no further upgrades will be allowed to prevent introducing more damage to 3scale.

More scenarios and verification steps are available in the Jira comments document link (due to the size of possible scenarios, a Word doc was used instead of steps here)

How it works

The requirements are set on the CSV. The new subscription controller watches the operator's subscription and discovers the latest CSV approved by OLM. It then reads the requirements from CSV annotations and creates config map with it.
APIManager controller is triggered on config map creation, it first checks if the config map is present at all, if not, it will requeue since the the config map presence is a must for it to continue (cm will be created even if no requiremets are set [empty values = no requirements])
Once the requirements config map is created, the requirements checks are performed, and the controller response is different based on different scenarios (more info in the doc linked in Jira). If requirements are confirmed, the APIM instance gets annotated with resource version of config map so that as long as resource version of config map isn't changed, the preflight isn't re run.

Subscription controller also controlls the state of the operator condition CR.
Operator Condition CR is used to prevent upgrades at the OLM level, even if the upgrade is approved by the user. 3scale Operator always blocks upgrades apart from situations when upgrade requirements are met.

Tweaks to be done and things to be considered:

  • checking db versions - via throwaways or go clients
  • improve logging
  • improve CSV locator
  • UNIT TESTS!
  • REBASE!
  • more?

@MStokluska MStokluska requested a review from a team as a code owner March 15, 2024 11:28
Copy link
Copy Markdown
Contributor

@carlkyrillos carlkyrillos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments but nothing major. Overall the code looks good to me and I completed all of the verification scenarios.

Comment thread controllers/apps/apimanager_status_reconciler.go Outdated
Comment thread config/requirements/operator-requirements.yaml Outdated
Comment thread controllers/apps/apimanager_controller.go Outdated
Comment thread controllers/apps/apimanager_status_reconciler.go Outdated
Comment thread config/rbac/role.yaml Outdated
Comment thread controllers/subscription/subscription_controller.go Outdated
Comment thread pkg/helper/catalogsource_client.go Outdated
Comment thread pkg/helper/k8s.go
Comment thread pkg/helper/podHelper.go Outdated
Comment thread pkg/helper/podHelper.go Outdated
Comment thread main.go
Comment thread main.go
@MStokluska MStokluska changed the title THREESCALE-10842 Preflights checks [WIP] THREESCALE-10842 Preflights checks Mar 22, 2024
Copy link
Copy Markdown
Contributor

@carlkyrillos carlkyrillos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MStokluska I left a few comments and performed the following four scenarios:

  • Scenario 3 (MySQL) single namespace scope
  • Scenario 4 (PostgreSQL) target namespace scope
  • Scenario 7 (PostgreSQL) cluster namespace scope
  • Scenario 8 (PostgreSQL) single namespace scope

They all worked except for Scenario 4 when done in target namespace mode (but it did work in single namespace mode). For background, I installed the operator to a namespace threescale-operator and updated the CSV to set the WATCH_NAMESPACE env var in the Deployment to threescale. I then created the APIManager in the namespace threescale but the operator logs were filled with RBAC errors and the APIManager wasn't reconciled.

Comment thread controllers/apps/apimanager_controller.go Outdated
Comment thread controllers/subscription/subscription_controller.go
Comment thread pkg/helper/podHelper.go
Comment thread pkg/helper/podHelper.go
Comment thread pkg/helper/requirements_common_helper_test.go Outdated
Comment thread Makefile Outdated
Comment thread doc/operator-user-guide.md Outdated
Copy link
Copy Markdown
Contributor

@carlkyrillos carlkyrillos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments about unused constants but feel free to ignore.
/lgtm

Comment thread controllers/subscription/subscription_controller.go Outdated
Comment thread pkg/helper/redis_database_version.go Outdated
Comment thread pkg/helper/requirements_common_helper.go Outdated
@carlkyrillos
Copy link
Copy Markdown
Contributor

/test test-e2e

1 similar comment
@MStokluska
Copy link
Copy Markdown
Contributor Author

/test test-e2e

@austincunningham
Copy link
Copy Markdown
Contributor

/lgtm

@MStokluska MStokluska merged commit 83a8303 into 3scale:master Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants