-
Notifications
You must be signed in to change notification settings - Fork 10
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
Apply velero crds manually during apps v0.36 upgrade #2008
Conversation
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.
The change is fine but it's a bit worrying that we aren't doing anything about the error messages. Even if it fails once and works the second time I think it would be good to at least know why.
log_info " - applying the Velero CRDs on wc" | ||
kubectl_do wc apply --server-side --force-conflicts -f "${ROOT}"/helmfile.d/upstream/vmware-tanzu/velero/crds | ||
|
||
helmfile_upgrade wc app=velero |
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.
The file seem to indicate that it upgrades the CRDs only but this will also upgrade Velero itself as well right?
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.
yes, upgrade both
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.
As a nit-pick this file could be renamed to "50-velero-upgrade.sh" to make it more clear that it is upgrading all of velero. But it is ok if you skip that.
I do not see any other action items that allows us to finish with/fix the apps v0.36 release (in a good way). I am open to suggestions. |
@simonklb although I wasn't able yet to fully test this myself, but here are some upstream issues for the same problem we had: I think, for us, applying the new CRDs via a script is a good solution for this release. Let me know if you think there is a better solution. @Pavan-Gunda, @viktor-f and @raviranjanelastisys please also check this workaround and let me now if you see any issues with it. |
What about overriding the kubectl image? |
I was thinking about that, but at some point in the future we will need to remember to update it (or return to default). This assuming that the new image will not create other issues, now or in the future. |
Yea, if you decide to override the image create an issue to revert it when a fix has been released. |
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.
Yes I'm ok with this fix
Warning
This is public repository, ensure not to disclose:
What kind of PR is this?
Required: Mark one of the following that is applicable:
Optional: Mark one or more of the following that are applicable:
Important
Breaking changes should be marked
kind/admin-change
orkind/dev-change
depending on typeCritical security fixes should be marked with
kind/security
What does this PR do / why do we need this PR?
During the upgrade to apps v0.36 the kubectl container used by Velero to apply the CRDs was failing with:
As this error seems to appear only sometimes I decided to apply the Velero CRDs using a migration script instead.
Additional information to reviewers
Screenshots
Checklist
NetworkPolicy Dashboard