use v1beta2 as the storage version of PingSource#4577
Conversation
knative-prow-robot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eclipselu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report
@@ Coverage Diff @@
## master #4577 +/- ##
=======================================
Coverage 81.31% 81.31%
=======================================
Files 290 290
Lines 8157 8157
=======================================
Hits 6633 6633
Misses 1126 1126
Partials 398 398 Continue to review full report at Codecov.
|
- flip storage version to v1beta2 in PingSource CRD - add post-install job to migrate existing PingSource objects to v1beta2
|
/test pull-knative-eventing-upgrade-tests |
|
/retest |
|
Upgrade test is failing because v1beta2 was not introduced in 0.19, this is expected. this test should work once we release 0.20. |
|
I think it's still premature to publish this PR, closing. |
|
I don't like the idea of postponing this PR. Since the reconciler migrated to v1beta2, for each reconcile the stored v1beta1 object is converted to v1beta2 and then back to b1beta1. This can potentially overload the webhook, especially when running in multi-tenant environment. @vaikas @cardil I'm not too familiar with the upgrade tests. Is there a way we can disable one particular test just for PingSource? |
do you mean we're flipping storage version to v1beta2 in 0.20 (the same version that we introduced v1beta2) ? |
|
@eclipselu: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
yes |
in this case how do we allow downgrading from 0.20 (introduced v1beta2 and set it as storage version) to 0.19 (no v1beta2)? |
|
That's a good point. Downgrading would require to run a job to do the conversion back. |
|
/hold @lionelvillard to follow-up for issues. Likely, we do not want to migrate the storage version for 0.20, target 0.21. |
knative-prow-robot
added
the
do-not-merge/hold
|
@cr22rc do you think that's fine to wait 0.21 for the post-install job? |
+1 was about to issue a hold due to exactly same reasons |
|
|
||
| // Package postinstall is a placeholder that allows us to pull in config files | ||
| // via go mod vendor. | ||
| package postinstall |
There was a problem hiding this comment.
@lionelvillard @pierDipi the PingSource migration is using v0.21.1, we should try to merge this with PRs like #4658 to make sure we have single post install directory. Do you think we should bring up in the eventing meeting?
There was a problem hiding this comment.
Do we remove the post-install directory after 0.20.0 is cut and add this commit back in?
There was a problem hiding this comment.
Do we remove the post-install directory after 0.20.0 is cut add this commit back in?
yes, like serving and as expected by the operator (@houshengbo, @Cynocracy)
There was a problem hiding this comment.
Yes, I'd expect a simple PR like #4658.
I guess we can discuss this asynchronously on an issue or PR.
Perhaps a document describing the post-install job story might be helpful.
|
The only thing we should avoid is some automation tools run by Prow will
have assumptions about the directory structure. But I will vote for not
having versions too.
…On Thu, Jan 7, 2021 at 10:38 AM Pierangelo Di Pilato < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In config/post-install/v0.21.0/placeholder.go
<#4577 (comment)>:
> +Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package postinstall is a placeholder that allows us to pull in config files
+// via go mod vendor.
+package postinstall
Yes, I'd expect a simple PR like #4658
<#4658>.
I guess we can discuss this asynchronously on an issue or PR.
Perhaps a document describing the post-install job story might be helpful.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4577 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACE6CNEOL7YCSHLO2X34JLLSYXIN5ANCNFSM4T7S4FFQ>
.
|
|
closing in favour of #4750, as the original fork branch was deleted by accident. |
7 participants
Fixes #4539
Proposed Changes
Release Note
Docs