Update manifest to deploy on Kubernetes 1.16. #376

jbrette · 2019-09-22T02:25:04Z

Which issue is resolved by this Pull Request:
Resolves #
Numerous kubeflow manifests can't deploy on Kubernetes 1.16 #375

Description of your changes:

Check the apiVersion where needed.
Update the kustomization variables definition where needed.
Add selector to Deployment where needed.
Rerun make generate and make test but DID NOT CHECKIN the .go files on purpose yet to not clutter the PR.

Checklist:

Unit tests have been rebuilt:
1. cd manifests/tests
2. make generate
3. make test

This change is

k8s-ci-robot · 2019-09-22T02:25:18Z

Hi @jbrette. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

jlewi · 2019-09-22T14:35:57Z

/ok-to-test
Thanks @jbrette Are the manifests still compatible with earlier versions of K8s?
The presubmits should be deploying Kubeflow on 1.12. So I think if the presubmits pass then that should confirm that it still works on 1.12. But it would be good to know if we forsee any backwards compatability issues.

If you send a PR to join the github org in kubeflow/internal-acls then you will be able to trigger the tests yourself.

- Update Deployment apiVersion to apps/v1 - Update Ingress apiVersion to networking.k8s.io

jbrette · 2019-09-23T05:14:49Z

@jlewi @kkasravi Seems to be working ok now.

I had to comment out the "apiVersion: networking.k8s.io/v1beta" in 4 or 5 places, and revert it back extension/v1beta1 because we are still testing with 1.12.7. That change was added in Kubernetes 1.14. Still the Roles are ready (they refer "extensions" and "networking.k8s.io"), so the amount of work next time will be much smaller.
It was kind of difficult to figure out what exact version is being used. kubeflow/kubeflow has tons of swagger.json. kubeflow/manifest contains kubernetes 1.17. We should probably open a bug so that somebody cleanup.
I was surprised that we are still using kubectl 1.10 (in the testing/images/DockerFile) and not something consistent such as 1.12.7

jbrette · 2019-09-23T14:01:43Z

/assign jlewi

jlewi · 2019-09-23T14:16:13Z

I was surprised that we are still using kubectl 1.10 (in the testing/images/DockerFile) and not something consistent such as 1.12.7

Yeah lots of room for cleanup and improvement in our eng infrastructure.

I had to comment out the "apiVersion: networking.k8s.io/v1beta" in 4 or 5 places, and revert it back extension/v1beta1 because we are still testing with 1.12.7. That change was added in Kubernetes 1.14. Still the Roles are ready (they refer "extensions" and "networking.k8s.io"), so the amount of work next time will be much smaller.

Could we potentially use an overlay to allow multi-version support? So we'd have an overlay that would change the apiVersion to networking.k8s.io and then folks deploying on 1.16 could include that overlay in the list of applied overlays?

jlewi · 2019-09-23T14:17:25Z

aws/istio-ingress/overlays/cognito/ingress.yaml

@@ -1,4 +1,4 @@
-apiVersion: extensions/v1beta1
+apiVersion: extensions/v1beta1 # networking.k8s.io/v1beta1


Do you want to add a TODO() about updating this?

@jlewi I will add an issue so that we remember to come back and fix them once with switch to Kubernete 1.14+. I'll add additional instruction on what to pay attention to in Role and ClusterRole definition.

jlewi · 2019-09-23T14:18:21Z

istio/istio-install/base/istio-noauth.yaml

+  selector:
+    matchLabels:
+      chart: gateways
+      heritage: Tiller


nit: can we get rid of the heritage label in the selector?

@jlewi I did not want to break anything. The labels, above an below the selectors have to match. Those currently contain Tiller everywhere. You will have to find somebody to run a script and do "grep -v Tiller" on the entire directory structure. We should do that in another PR.

@jlewi Removal of Tiller has been done incrementally in this PR #378

jlewi · 2019-09-23T14:18:36Z

istio/istio-install/base/istio-noauth.yaml

+  selector:
+    matchLabels:
+      chart: gateways
+      heritage: Tiller


nit can we get rid of the heritage label in the selector?

not in this PR

@jlewi Removal of Tiller has been done incrementally in this PR #378

jlewi · 2019-09-23T14:19:00Z

istio/istio-install/base/istio-noauth.yaml

+    matchLabels:
+      app: jaeger
+      chart: tracing
+      heritage: Tiller


nit: Here and everywhere else get rid of heritage label in selector

@jlewi Removal of Tiller has been done incrementally in this PR #378

jbrette

I was surprised that we are still using kubectl 1.10 (in the testing/images/DockerFile) and not something consistent such as 1.12.7

Yeah lots of room for cleanup and improvement in our eng infrastructure.

I had to comment out the "apiVersion: networking.k8s.io/v1beta" in 4 or 5 places, and revert it back extension/v1beta1 because we are still testing with 1.12.7. That change was added in Kubernetes 1.14. Still the Roles are ready (they refer "extensions" and "networking.k8s.io"), so the amount of work next time will be much smaller.

Could we potentially use an overlay to allow multi-version support? So we'd have an overlay that would change the apiVersion to networking.k8s.io and then folks deploying on 1.16 could include that overlay in the list of applied overlays?

We can not.

Kustomize is using the apiVersion field and the kind field for match resources between the base and the overlay,
It could be a real problem in some of the kustomize params.xxx. But we are lucky, most of those configuration files are using Ingress, not (Ingress + apiVersion).

jlewi · 2019-09-23T23:59:26Z

Thanks
/lgtm
/approve

k8s-ci-robot · 2019-09-24T00:00:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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:

~~OWNERS~~ [jlewi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…eway Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

…gressgateway (kubeflow#383)" This reverts commit 8196f65.

googlebot added the cla: yes label Sep 22, 2019

k8s-ci-robot requested review from Jeffwan and jlewi September 22, 2019 02:25

k8s-ci-robot added needs-ok-to-test size/L labels Sep 22, 2019

k8s-ci-robot added size/XL size/XXL and removed size/L size/XL labels Sep 22, 2019

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Sep 22, 2019

Kubernetes 1.16: Update Deployment and Ingress objects

2efd273

- Update Deployment apiVersion to apps/v1 - Update Ingress apiVersion to networking.k8s.io

k8s-ci-robot added size/L size/XL and removed size/XXL size/L labels Sep 22, 2019

jbrette added 7 commits September 22, 2019 23:57

Kubernetes 1.16: Solve selector issue in Deployments

f01440c

Kubernetes 1.16: Solve selector issue in StatefulSets

b41e242

Kubernetes 1.16: Regenerate go code after updating objects

ac70712

Kubernetes 1.16: Update RBAC setup

12aac02

Kubernetes 1.16: Regenerate after RBAC changes

1383bf2

Kubernetes 1.16: Adjust RBAC

af9ec2d

Kubernetes 1.16: Regenerate after RBAC changes.

ab0b09b

k8s-ci-robot assigned jlewi Sep 23, 2019

jlewi reviewed Sep 23, 2019

View reviewed changes

jbrette commented Sep 23, 2019

View reviewed changes

k8s-ci-robot added the lgtm label Sep 23, 2019

k8s-ci-robot added the approved label Sep 24, 2019

k8s-ci-robot merged commit 0842eb2 into kubeflow:master Sep 24, 2019

jbrette deleted the kube16 branch September 24, 2019 00:56

yanniszark added a commit to arrikto/kubeflow-manifests that referenced this pull request Sep 24, 2019

kfdef: generic: revert kubeflow#376 because it breaks istio-egressgat…

4549f38

…eway Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

yanniszark mentioned this pull request Sep 24, 2019

kfdef: generic: revert #376 #383

Merged

1 task

k8s-ci-robot pushed a commit that referenced this pull request Sep 24, 2019

kfdef: generic: revert #376 because it breaks istio-egressgateway (#383)

8196f65

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

jbrette added a commit to prorates/manifests that referenced this pull request Oct 7, 2019

Revert "kfdef: generic: revert kubeflow#376 because it breaks istio-e…

69da4ae

…gressgateway (kubeflow#383)" This reverts commit 8196f65.

jbrette added a commit to prorates/manifests that referenced this pull request Oct 11, 2019

Revert "kfdef: generic: revert kubeflow#376 because it breaks istio-e…

63dc14b

…gressgateway (kubeflow#383)" This reverts commit 8196f65.

lwilson mentioned this pull request Mar 9, 2020

Provide branch with v1.16 and GPU Operator dell/omnia#15

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update manifest to deploy on Kubernetes 1.16. #376

Update manifest to deploy on Kubernetes 1.16. #376

jbrette commented Sep 22, 2019 •

edited by jlewi

Loading

k8s-ci-robot commented Sep 22, 2019

jlewi commented Sep 22, 2019

jbrette commented Sep 23, 2019

jbrette commented Sep 23, 2019

jlewi commented Sep 23, 2019

jlewi Sep 23, 2019

jbrette Sep 23, 2019

jlewi Sep 23, 2019

jbrette Sep 23, 2019

jbrette Sep 23, 2019

jlewi Sep 23, 2019

jbrette Sep 23, 2019

jbrette Sep 23, 2019

jlewi Sep 23, 2019

jbrette Sep 23, 2019

jbrette left a comment

jlewi commented Sep 23, 2019

k8s-ci-robot commented Sep 24, 2019

		@@ -1,4 +1,4 @@
		apiVersion: extensions/v1beta1
		apiVersion: extensions/v1beta1 # networking.k8s.io/v1beta1

Update manifest to deploy on Kubernetes 1.16. #376

Update manifest to deploy on Kubernetes 1.16. #376

Conversation

jbrette commented Sep 22, 2019 • edited by jlewi Loading

k8s-ci-robot commented Sep 22, 2019

jlewi commented Sep 22, 2019

jbrette commented Sep 23, 2019

jbrette commented Sep 23, 2019

jlewi commented Sep 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrette left a comment

Choose a reason for hiding this comment

jlewi commented Sep 23, 2019

k8s-ci-robot commented Sep 24, 2019

jbrette commented Sep 22, 2019 •

edited by jlewi

Loading