Skip to content
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.

Sign up for GitHub

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

Jump to bottom

common: Add Istio v1.16.0 manifests #2327

Merged
merged 6 commits into from
Nov 24, 2022
Merged

common: Add Istio v1.16.0 manifests #2327

merged 6 commits into from
Nov 24, 2022

Conversation

apo-ger
Copy link
Contributor

@apo-ger apo-ger commented Nov 22, 2022 •
edited
Loading

Changes:

  • Upgrade Istio to 1.16.0 to work with K8s 1.25
  • Update kustomization file in example to deploy istio-1-16
  • Update Istio GH action script to install Istio 1.16 for testing
  • Introduce a temporary workaround to remove PodDisruptionBudget resources manually with yq before deploying Istio manifests
  • Update README file with instructions

Refs: #2325

@apo-ger apo-ger mentioned this pull request Nov 22, 2022
Closed
@apo-ger apo-ger force-pushed the feature-arrikto-apoger-istio-1.16 branch 2 times, most recently from 1d273d2 to 846e007 Compare November 22, 2022 16:29
@apo-ger apo-ger changed the title common: Add Istio v1.6.0 manifests common: Add Istio v1.16.0 manifests Nov 22, 2022
@juliusvonkohout
Copy link
Member

@apo-ger we would like to switch to istio-cni as discussed with @kimwnasptd to get rootless istio and apply restricted podsecuritystandards in the future. Here is the simple guide https://istio.io/latest/docs/setup/additional-setup/cni/

@apo-ger apo-ger force-pushed the feature-arrikto-apoger-istio-1.16 branch from bb0dce1 to 20eb046 Compare November 23, 2022 14:07
@apo-ger apo-ger mentioned this pull request Nov 24, 2022
Merged
@kimwnasptd
Copy link
Member

kimwnasptd commented Nov 24, 2022 •
edited
Loading

@juliusvonkohout let's have a separate issue for integrating istio-cni in manifests. This way we can give a heads up to distros as well and make sure everyone is in sync with such a change and expose any technical changes users want to know.

Also, I'd like to not block this PR with the istio-cni discussion since it's needed for running other K8s 1.25 tests #2330 (comment) #2331 (comment)

@kimwnasptd
Copy link
Member

@apo-ger in the same spirit of #2330 (comment) #2331 (comment) let's also run the workflows that depend on Istio when the Istio version changes

@kimwnasptd
Copy link
Member

I think the reason that the workflows weren't triggered was because of the path:

      - apps/centraldashboard/upstream/**
      - /tests/gh-actions/install_istio.sh

Let's try to use relative paths for the install_istio.sh file as well to see if this fixes the issue

@apo-ger apo-ger force-pushed the feature-arrikto-apoger-istio-1.16 branch from 52cec5e to 3757c53 Compare November 24, 2022 13:30
@apo-ger
common: Add Istio v1.16.0 manifests
07f3a3f 
Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
@apo-ger
Update kustomization file in example to deploy istio-1-16
46bcfcb 
Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
@apo-ger
tests: Update install Istio GH action script
b20d841 
Use Istio 1.16 for testing

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
@apo-ger
common: Remove PodDisruptionBudget resources for Istio
a45a129 
Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
@apo-ger
Update README
c4c6509 
Use the --cluster-specific flag when generating Istio 1.16 manifests
for K8s-1.25. See:
* istio/istio#41220

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
@apo-ger apo-ger force-pushed the feature-arrikto-apoger-istio-1.16 branch from 3757c53 to 9482ba2 Compare November 24, 2022 14:10
@apo-ger
Copy link
Contributor Author

apo-ger commented Nov 24, 2022 •
edited
Loading

Re-testing the GH action workflows since we merged #2331

I excluded the Katib and Kserve workflows here, similar to what we did in #2331 (comment)

@apo-ger
tests: Update GH Action workflows
404c57e 
Trigger the workflows when the Istio version changes

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
@apo-ger apo-ger force-pushed the feature-arrikto-apoger-istio-1.16 branch from 9482ba2 to 404c57e Compare November 24, 2022 14:14
@kimwnasptd
Copy link
Member

/lgtm
/approve

@google-oss-prow google-oss-prow bot added the lgtm label Nov 24, 2022
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apo-ger, kimwnasptd

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 0822809 into kubeflow:master Nov 24, 2022
@apo-ger apo-ger mentioned this pull request Nov 24, 2022
Merged
kevin85421 pushed a commit to juliusvonkohout/manifests that referenced this pull request Feb 28, 2023
@apo-ger @kevin85421
common: Add Istio v1.16.0 manifests (kubeflow#2327)
e238b49 
* common: Add Istio v1.16.0 manifests

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>

* Update kustomization file in example to deploy istio-1-16

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>

* tests: Update install Istio GH action script

Use Istio 1.16 for testing

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>

* common: Remove PodDisruptionBudget resources for Istio

Istio 1.6.0 generated manifests include some policy/v1
PodDisruptionBudget resources that we need to remove. See:
- istio/istio#12602
- istio/istio#24000

The current manifests utilize two "delete" patches:
- common/istio-1-16/istio-install/base/patches/remove-pdb.yaml
- common/istio-1-16/cluster-local-gateway/base/patches/remove-pdb.yaml

However these patches do not work with kustomize v3.2.0. The root
cause is that v3.2.0 doesn't have the appropriate openapi schema for
the policy/v1 API version resources. This is fixed in kustomize v4+.
See:
- kubernetes-sigs/kustomize#3694 (comment)
- kubernetes-sigs/kustomize#4495

Changes:
- We disable the delete patches until we upgrade to kustomize v4+.
  - tracked in: kubeflow#1797

- As a temporary workaraound we remove PodDisruptionBudget resources
  manually with yq before deploying Istio manifests.

- Update README file with instructions.

Refs: kubeflow#2325

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>

* Update README

Use the --cluster-specific flag when generating Istio 1.16 manifests
for K8s-1.25. See:
* istio/istio#41220

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>

* tests: Update GH Action workflows

Trigger the workflows when the Istio version changes

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PatrickXYS PatrickXYS Awaiting requested review from PatrickXYS

@StefanoFioravanzo StefanoFioravanzo Awaiting requested review from StefanoFioravanzo

Assignees

@kimwnasptd kimwnasptd

Labels
approved lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apo-ger @juliusvonkohout @kimwnasptd