-
Notifications
You must be signed in to change notification settings - Fork 196
K8SPXC-1584: replace typo NAMESPASE with NAMESPACE #1949
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
Conversation
@@ -279,7 +279,7 @@ func (c *Node) LogCollectorContainer(spec *api.LogCollectorSpec, logPsecrets str | |||
Value: "/var/lib/mysql", | |||
}, | |||
{ | |||
Name: "POD_NAMESPASE", | |||
Name: "POD_NAMESPACE", |
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.
@franklouwers thank you for PR! but we can remove these var only for crVersion >= 1.20.0 like https://github.com/percona/percona-xtradb-cluster-operator/pull/1949/files#diff-4c64a958f53a8f3616564c42beb08dfeeaee83a9ae0a13dab3a41cb375998748L407
We need to have it to avoid cluster restart as soon as you update operator image.
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.
@hors I am sorry, I have no idea how to do that. I don't know (nor really want to know) how the operator works. I've opened an issue for an embarrassing typo that got in, about 3 months ago. Last week you asked me to prep a PR. I just did.
As the typo is in an envvar, which will force a restart, then yes, I assume it will restart. If there's a mechanism to prevent that, I don't know how to do that. It doesn't seem to be in the public documentation.
You mention crVersion: I see it's currently 1.17.0. Should I just change that to 1.20.0 in all places? Why 1.20 btw and not 1.18?
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.
@franklouwers basically we do these kind of version checks because we don't want to trigger restarts in all clusters controlled by the operator right after you update the image of operator deployment. Operator might be controlling a lot of clusters and users should be able to control the upgrade process.
you can find an example how to do this with a version check here:
if cr.CompareVersionWith("1.17.0") >= 0 { |
versions <1.17.0 should use the old name for env var and >=1.17.0 should use the new 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.
@franklouwers any updates from your side for these comments?
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.
@gkech was hoping to quickly get an embarrassing typo out of the way. Didn't know I had to guess version numbers and do strange dances. Closing this PR, as I have no time for this, just tried to get a quick win in terms of usability.
commit: 00b60e7 |
replace typo NAMESPASE with NAMESPACE
Problem:
One of the ENVVARs set by the operator is the POD_NAMESPASE envvar, which contains the namespace the pod is deployed to.
For correct spelling and consistency, this should be POD_NAMESPACE instead
Closes #1865
Cause:
Likely a typo.
Solution:
Fix the typo.
CHECKLIST
Jira
Needs Doc
) and QA (Needs QA
)?Tests
compare/*-oc.yml
)?Config/Logging/Testability