-
Notifications
You must be signed in to change notification settings - Fork 884
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
Update kubeflow/kubeflow manifests from v1.9.0-rc.0 #2734
Update kubeflow/kubeflow manifests from v1.9.0-rc.0 #2734
Conversation
Signed-off-by: Kimonas Sotirchos <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@kimwnasptd looks good on a first glimpse. The failing test is the only major thing. |
@juliusvonkohout the failure is unrelated to this PR though. I see the following
Are the |
It looks like a manifest file was introduced in the manifests repo, but was not part of the source manifests Is this the intended behavior? |
It is used to patch the logout https://github.com/kubeflow/manifests/blob/v1.9-branch/common/oidc-client/oauth2-proxy/components/central-dashboard/patches/deployment.logout-url.yaml we need to find a new place for that patch, because it should be maintained within kubeflow/manifests. You can also readd it for now and we tackle that in RC.2. CC @kromanow94 |
| Tensorboards Web App | apps/tensorboard/tensorboards-web-app/upstream | [v1.8.0](https://github.com/kubeflow/kubeflow/tree/v1.8.0/components/crud-web-apps/tensorboards/manifests) | | ||
| Volumes Web App | apps/volumes-web-app/upstream | [v1.8.0](https://github.com/kubeflow/kubeflow/tree/v1.8.0/components/crud-web-apps/volumes/manifests) | | ||
| Notebook Controller | apps/jupyter/notebook-controller/upstream | [v1.9.0-rc.0](https://github.com/kubeflow/kubeflow/tree/v1.9.0-rc.0/components/notebook-controller/config) | | ||
| PVC Viewer Controller | apps/pvcviewer-roller/upstream | [v1.9.0-rc.0](https://github.com/kubeflow/kubeflow/tree/v1.9.0-rc.0/components/pvcviewer-controller/config) | |
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.
Is this a typo?
| PVC Viewer Controller | apps/pvcviewer-roller/upstream | [v1.9.0-rc.0](https://github.com/kubeflow/kubeflow/tree/v1.9.0-rc.0/components/pvcviewer-controller/config) | | |
| PVC Viewer Controller | apps/pvcviewer-controller/upstream | [v1.9.0-rc.0](https://github.com/kubeflow/kubeflow/tree/v1.9.0-rc.0/components/pvcviewer-controller/config) | |
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 let's fix it
The error is in an overlay for oauth2 for central dashboard. I thought this was tested in kubeflow/kubeflow repo before, wasn't it? EDIT: I got the whole thread now. What was the final decision about it? It looks like a simple change, so what's the ETA to get this done? |
I am in favour of just re-adding it (5 minutes of work) for now and fixing it in RC.2. RC.2 can be used for that and upgrading istio and the sometimes slow oauth2-proxy installation. Let's resynchronize on Monday for that. |
@kimwnasptd Lets close this. It is syncing into the wrong branch. We need to sync to master first and then from master into 1.9. @rimolive Can you run the script to create a new PR against the master branch and undelete /manifests/manifests/apps/centraldashboard/upstream/overlays/oauth2-proxy. Then we need to synchronize the 1.9 branch to master and you can tag RC.1 this week with #2739 |
As per Julius suggestion, will close this PR and create a new one with the missing manifest. /close |
@rimolive: Closed this PR. 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-sigs/prow repository. |
@rimolive @juliusvonkohout aside the PR target issue (the PR should obviously be into It's probably better if we just move anything that was manually added to the "upstream" folder (this |
@thesuperzapper I agree with that. @juliusvonkohout let's work on this in future releases, or check if it's possible to make that change for 1.9. |
Yes i hope so for 1.9 RC.2 |
This PR updates the manifests from
kubeflow/kubeflow
components in the manifests for 1.9.The manifests are copied from https://github.com/kubeflow/kubeflow/releases/tag/v1.9.0-rc.0
/cc @thesuperzapper @juliusvonkohout @rimolive