-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌱 Change the status into progressing
if the upgraded pg instance is not ready
#1358
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: myan <[email protected]>
progressing
if the upgraded pg instance is not ready
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ldpliu, yanmxa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
// Deprecated: The built-in PostgreSQL name `multicluster-global-hub-postgres` has been changed to | ||
// `multicluster-global-hub-postgresql` starting from Global Hub release 2.13. | ||
if dcs.Name != "multicluster-global-hub-postgres" && !desiredComponents.Has(dcs.Name) { | ||
log.Warnf("processing components: unsupported %s", dcs.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not we need to log warning for unsupported component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we don't need to log the information, the component all created by ourself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ldpliu Do you remember why we add this logic in the past?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's added in this pr: 1e9f112#diff-9f7d6063a139ba593684a7dd149b487b3decb1a83c9008e8dc45110adaac497aR293
I think it's not necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the upgrade environment, the component should also contain multicluster-global-hub-postgres
. We may need to ignore this one since we won't use it. Does it make sense?
operator/pkg/controllers/meta.go
Outdated
// Deprecated: The built-in PostgreSQL name `multicluster-global-hub-postgres` has been changed to | ||
// `multicluster-global-hub-postgresql` starting from Global Hub release 2.13. | ||
// If the new instance is not ready, we should keep the phase as progressing. | ||
newPgIsnstance := false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/s/newPgIsnstance/newPgInstance/g
Signed-off-by: myan <[email protected]>
New changes are detected. LGTM label has been removed. |
Signed-off-by: myan <[email protected]>
|
Summary
Related issue(s)
Fixes #
Tests
make unit-tests
.make integration-test
.make e2e-test-all
.