-
Notifications
You must be signed in to change notification settings - Fork 600
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
[post jobs] Like serving use generatedName #5664
[post jobs] Like serving use generatedName #5664
Conversation
Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matzew 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 |
/hold /assign @dprotaso a change like on your serving PR for the generated name. For the operator, since serving uses this |
Codecov Report
@@ Coverage Diff @@
## main #5664 +/- ##
=======================================
Coverage 82.68% 82.68%
=======================================
Files 200 200
Lines 6253 6253
=======================================
Hits 5170 5170
Misses 751 751
Partials 332 332 Continue to review full report at Codecov.
|
@dprotaso, @houshengbo, @lionelvillard any comments 🙏 ? |
Right now we still have the storage version migration job in serving but it's an effective NOOP until we change the storage version of some CRDs which we haven't done in a while. We currently use generateName it's possible to re-apply this job over and over when upgrading & downgrading. Given that I thought maybe in serving we switch to versioning the jobs. Maybe be deleting and re-introducing the post-install jobs when needed. But it looks like you want to go the other way. There's also the caveat of some post-install jobs being optional (ie. default-domain) so we continue offering those. Either way switching to generate name makes sense in cause you want to re-apply or downgrade etc. I'm curious to hear @houshengbo's take given the operator has been dealing with both the versioned post-install jobs (from eventing) and the non-versioned ones. (from serving). |
@houshengbo WDYT? |
Ah, I thought it was intentionally done as is currently (e.g. have them always, even as NOOP). In eventing we had be doing this before: deleting and re-introducing - but it was forgotten a for a few releases, which shows the downside of the "delete" and "re-introduce" model |
Hi folks, thanks for reminding me. |
/unhold |
A follow up, like serving, will be done in here: #5694 @lionelvillard mind getting me some LGTM? |
/lgtm |
Signed-off-by: Matthias Wessendorf mwessend@redhat.com
Proposed Changes
generatedName
for the upgrade job. See: https://github.com/knative/serving/pull/8514/files#r447739231. Also updating the job name and the labels, like: Update the job name and label into storage-version-migration-serving serving#8580Pre-review Checklist
Release Note
Docs