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
Add finalizers to roles, change argo executor to k8sapi #276
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Hi @blublinsky. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
/assign @zhenghuiwang |
/assign @richardsliu for tf-jobs config |
@zhenghuiwang: GitHub didn't allow me to assign the following users: config, for, tf-jobs. Note that only kubeflow members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. 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. |
@blublinsky Also a couple of things:
|
@richardsliu This changes are necessary for OpenShift/IBM execution, due to their security constraints. As operator is destroying, its using finalizers and need to have access to it. |
@blublinsky Can you at least update katib-v1alpha2, since katib-v1alpha1 will be deprecated soon (in about a month)? |
[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 |
This change is my best guess, I did not try it and v2 is significantly different |
/ok-to-test |
Generally lgtm. Can you resolve the conflict and fix the tests? |
Conflicts resolved |
Conflicts resolved
Boris Lublinsky
FDP Architect
boris.lublinsky@lightbend.com
https://www.lightbend.com/
… On Aug 22, 2019, at 10:15 PM, Richard Liu ***@***.***> wrote:
Generally lgtm. Can you resolve the conflict and fix the tests?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#276?email_source=notifications&email_token=AGA5MG3MAPQGLY2FX7ZSHQTQF5W5VA5CNFSM4ILZHVP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD47EPMQ#issuecomment-524175282>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGA5MG3G43PF6ORCDIMQMB3QF5W5VANCNFSM4ILZHVPQ>.
|
Looking at the tests and slightly confused. The test that fails is FAIL: TestTfJobOperatorOverlaysIstio (0.05s) |
You need to fix files like these: They can be run locally by doing |
Thanks Richard |
@blublinsky: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
OK,
The initial problem is fixed.
The new one is:
Error: couldn't apply KfApp: (kubeflow.error): Code 500 with message: kfApp Apply failed
for kustomize: (kubeflow.error): Code 500 with message: couldn't create resources from seldon-core-operator Error: StatefulSet in
version "v1" cannot be handled as a StatefulSet: v1.StatefulSet.Spec: v1.StatefulSetSpec.VolumeClaimTemplates: []v1.PersistentVol
umeClaim: decode slice: expect [ or n, but found {, error found in #10 byte of ...|mplates":{"metadata"|..., bigger context ...|bh
ook-server-secret"}}]}},"volumeClaimTemplates":{"metadata":{"labels":{"app.kubernetes.io/component|…
This is definitely not me
Boris Lublinsky
FDP Architect
boris.lublinsky@lightbend.com
https://www.lightbend.com/
… On Aug 23, 2019, at 12:03 PM, Kubernetes Prow Robot ***@***.***> wrote:
@blublinsky <https://github.com/blublinsky>: The following test failed, say /retest to rerun them all:
Test name Commit Details Rerun command
kubeflow-manifests-presubmit d4c4b6a <d4c4b6a> link <https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubeflow_manifests/276/kubeflow-manifests-presubmit/1164970103964438528/> /test kubeflow-manifests-presubmit
Full PR test history <https://prow.k8s.io/pr-history?org=kubeflow&repo=manifests&pr=276>. Your PR dashboard <https://gubernator.k8s.io/pr/blublinsky>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/kubeflow/manifests/issues?q=is:issue+is:open> when you hit one in your PR.
<https://git.k8s.io/community/contributors/guide/pull-requests.md> <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> <https://go.k8s.io/bot-commands>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#276?email_source=notifications&email_token=AGA5MGYETSHL5QQ72CLS4YDQGAYA3A5CNFSM4ILZHVP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5BCFBI#issuecomment-524427909>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGA5MG5MHTJ6NWZUWX2GK4TQGAYA3ANCNFSM4ILZHVPQ>.
|
This is a kustomize-2.0.3 bug in StatefulSet, you need to add
volumeClaimTemplates: []
at the end. Look at manifests/gcp/iap-ingress/base/stateful-set.yaml for an example
From: Boris Lublinsky <notifications@github.com>
Reply-To: kubeflow/manifests <reply@reply.github.com>
Date: Friday, August 23, 2019 at 12:26 PM
To: kubeflow/manifests <manifests@noreply.github.com>
Cc: Kam Kasravi <kamkasravi@yahoo.com>, Review requested <review_requested@noreply.github.com>
Subject: Re: [kubeflow/manifests] Add finalizers to roles, change argo executor to k8sapi (#276)
OK,
The initial problem is fixed.
The new one is:
Error: couldn't apply KfApp: (kubeflow.error): Code 500 with message: kfApp Apply failed
for kustomize: (kubeflow.error): Code 500 with message: couldn't create resources from seldon-core-operator Error: StatefulSet in
version "v1" cannot be handled as a StatefulSet: v1.StatefulSet.Spec: v1.StatefulSetSpec.VolumeClaimTemplates: []v1.PersistentVol
umeClaim: decode slice: expect [ or n, but found {, error found in #10 byte of ...|mplates":{"metadata"|..., bigger context ...|bh
ook-server-secret"}}]}},"volumeClaimTemplates":{"metadata":{"labels":{"app.kubernetes.io/component|…
This is definitely not me
Boris Lublinsky
FDP Architect
boris.lublinsky@lightbend.com
https://www.lightbend.com/
On Aug 23, 2019, at 12:03 PM, Kubernetes Prow Robot ***@***.***> wrote:
@blublinsky <https://github.com/blublinsky>: The following test failed, say /retest to rerun them all:
Test name Commit Details Rerun command
kubeflow-manifests-presubmit d4c4b6a <d4c4b6a> link <https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubeflow_manifests/276/kubeflow-manifests-presubmit/1164970103964438528/> /test kubeflow-manifests-presubmit
Full PR test history <https://prow.k8s.io/pr-history?org=kubeflow&repo=manifests&pr=276>. Your PR dashboard <https://gubernator.k8s.io/pr/blublinsky>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/kubeflow/manifests/issues?q=is:issue+is:open> when you hit one in your PR.
<https://git.k8s.io/community/contributors/guide/pull-requests.md> <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> <https://go.k8s.io/bot-commands>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#276?email_source=notifications&email_token=AGA5MGYETSHL5QQ72CLS4YDQGAYA3A5CNFSM4ILZHVP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5BCFBI#issuecomment-524427909>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGA5MG5MHTJ6NWZUWX2GK4TQGAYA3ANCNFSM4ILZHVPQ>.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Its already there
Boris Lublinsky
FDP Architect
boris.lublinsky@lightbend.com
https://www.lightbend.com/
… On Aug 24, 2019, at 12:25 AM, Kam Kasravi ***@***.***> wrote:
This is a kustomize-2.0.3 bug in StatefulSet, you need to add
volumeClaimTemplates: []
at the end. Look at manifests/gcp/iap-ingress/base/stateful-set.yaml for an example
From: Boris Lublinsky ***@***.***>
Reply-To: kubeflow/manifests ***@***.***>
Date: Friday, August 23, 2019 at 12:26 PM
To: kubeflow/manifests ***@***.***>
Cc: Kam Kasravi ***@***.***>, Review requested ***@***.***>
Subject: Re: [kubeflow/manifests] Add finalizers to roles, change argo executor to k8sapi (#276)
OK,
The initial problem is fixed.
The new one is:
Error: couldn't apply KfApp: (kubeflow.error): Code 500 with message: kfApp Apply failed
for kustomize: (kubeflow.error): Code 500 with message: couldn't create resources from seldon-core-operator Error: StatefulSet in
version "v1" cannot be handled as a StatefulSet: v1.StatefulSet.Spec: v1.StatefulSetSpec.VolumeClaimTemplates: []v1.PersistentVol
umeClaim: decode slice: expect [ or n, but found {, error found in #10 byte of ...|mplates":{"metadata"|..., bigger context ...|bh
ook-server-secret"}}]}},"volumeClaimTemplates":{"metadata":{"labels":{"app.kubernetes.io/component|…
This is definitely not me
Boris Lublinsky
FDP Architect
***@***.***
https://www.lightbend.com/
> On Aug 23, 2019, at 12:03 PM, Kubernetes Prow Robot ***@***.***> wrote:
>
> @blublinsky <https://github.com/blublinsky>: The following test failed, say /retest to rerun them all:
>
> Test name Commit Details Rerun command
> kubeflow-manifests-presubmit d4c4b6a <d4c4b6a> link <https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubeflow_manifests/276/kubeflow-manifests-presubmit/1164970103964438528/> /test kubeflow-manifests-presubmit
> Full PR test history <https://prow.k8s.io/pr-history?org=kubeflow&repo=manifests&pr=276>. Your PR dashboard <https://gubernator.k8s.io/pr/blublinsky>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/kubeflow/manifests/issues?q=is:issue+is:open> when you hit one in your PR.
>
> <https://git.k8s.io/community/contributors/guide/pull-requests.md> <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> <https://go.k8s.io/bot-commands>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <#276?email_source=notifications&email_token=AGA5MGYETSHL5QQ72CLS4YDQGAYA3A5CNFSM4ILZHVP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5BCFBI#issuecomment-524427909>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGA5MG5MHTJ6NWZUWX2GK4TQGAYA3ANCNFSM4ILZHVPQ>.
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub, or mute the thread.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#276?email_source=notifications&email_token=AGA5MG63T5TJFREC3VU4HKTQGDA4PA5CNFSM4ILZHVP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5BZCYQ#issuecomment-524521826>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGA5MG7GYFKXPJOPRODJAJDQGDA4PANCNFSM4ILZHVPQ>.
|
I believe Clive Seldon submitted a fix |
The fix was in my version of code, the one that failed
Boris Lublinsky
FDP Architect
boris.lublinsky@lightbend.com
https://www.lightbend.com/
… On Aug 26, 2019, at 8:13 PM, Kam Kasravi ***@***.***> wrote:
I believe Clive Seldon submitted a fix
https://github.com/kubeflow/manifests/pull/322/files#diff-a8346fa7ae5f317402b5ef75772924abR7 <https://github.com/kubeflow/manifests/pull/322/files#diff-a8346fa7ae5f317402b5ef75772924abR7>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#276?email_source=notifications&email_token=AGA5MG4YXGYQNJNSXA2ZAOLQGR5UDA5CNFSM4ILZHVP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5GEWGI#issuecomment-525093657>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGA5MG5UGX3G2OJP3JMNT3TQGR5UDANCNFSM4ILZHVPQ>.
|
/assign @jlewi @blublinsky sorry this PR is taking so long; do you want to try to update? The RBAC role changes seem like pretty straightforward changes. I apologize this PR fell through the cracks and is taking so long. Here's my suggestion to get this PR fixed and merged
|
* update notebook dockerfiles * update requirements * Update the notebook dropdown * Update website * Add TF changes * Drop the date from the TF tags
Which issue is resolved by this Pull Request:
Resolves #
deployment issues on OpenShift
Description of your changes:
Add finalizers to roles
Checklist:
cd manifests/tests
make generate
make test
This change is