-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Upgraded e2e/apimachinery/aggregator to 1.10 Sample API server #68300
Conversation
/sig api-machinery |
/hold @yliaog how can the sample-apiserver image be kubernetes 1.9 based? the Dockerfile still has not been updated! did someone push a image without a change being merged in k/k repository? https://github.com/kubernetes/kubernetes/blob/master/test/images/sample-apiserver/Dockerfile#L26 |
@dims yes, that's my fault - I've generated and uploaded the image while I was working on upgrading the test, the yliao took over, but the image was uploaded. I'll fix it soon. |
That said, we also discussed having a test for 1.9, another one for 1.10, and so on for at least the -2 versions we support by policy. So in your opinion, which version should be in the master docker file? |
@fedebongio i am good whatever you all come up with :) (logically the image name should probably have version number of k/ and the docker file should probably start from 1.0 and get bumped when the image gets updated.) |
/assign @fedebongio |
@fedebongio: GitHub didn't allow me to assign the following users: fedebongio. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
Instructions 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. |
1.9 is about to become unsupported once 1.12 is released. should we be targeting something newer? |
I think the ideal is to have a test for the 3 supported versions that we support for policy. And yes, this is taking so long, that by the time we merge this one, will be deprecated (will always be better than 1.7...). |
Submitting #68545 |
/cc @cheftako |
/retest |
Depend on #69239 |
after some in-depth investigations, it turns out the 1.10 sample-apiserver would need the following rbac permissions (in addition to the system:auth-delegator role). I'm updating the PR to add these. ================================
|
/retest |
1 similar comment
/retest |
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.
/lgtm
/remove hold |
/hold cancel |
/approve |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheftako, janetkuo, yliaog 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 |
/retest |
1 similar comment
/retest |
What this PR does / why we need it:
Upgraded the e2e test "Should be able to support the 1.10 Sample API Server using the current Aggregator" from 1.7
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #63622
Special notes for your reviewer:
Release note: