Skip to content

Feat: Migrated to Istio 1.26.1 and merged istio and istio cni#3153

Merged
google-oss-prow[bot] merged 34 commits into
kubeflow:masterfrom
madmecodes:merge-istio-folders
Jun 18, 2025
Merged

Feat: Migrated to Istio 1.26.1 and merged istio and istio cni#3153
google-oss-prow[bot] merged 34 commits into
kubeflow:masterfrom
madmecodes:merge-istio-folders

Conversation

@madmecodes

@madmecodes madmecodes commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

✏️ Summary of Changes

  • Merged istio-1-24 and istio-cni-1-24 into unified common/istio
  • CNI-enabled Istio as default configuration
  • Minimal insecure overlay for environments without CNI support
  • Updated from Istio 1.24.3 to 1.26.1
  • Updated all references across codebase

Installation Options

# Default CNI (recommended)
kubectl apply -k common/istio/istio-install/overlays/oauth2-proxy

# Insecure (CNI-disabled)
kubectl apply -k common/istio/istio-install/overlays/insecure

Verification

CNI Installation (Default):

kubectl logs -n istio-system deployment/istiod | grep cni
# Output: CniNamespace: istio-system

kubectl label namespace default istio-injection=enabled
kubectl create deployment test --image=nginx
kubectl get pod -l app=test -o yaml | grep "name: istio-proxy"
# Output: Shows istio-proxy sidecar injected

Insecure Installation:

kubectl logs -n istio-system deployment/istiod | grep cni  
# Output: CniNamespace: ""

kubectl create deployment test --image=nginx
kubectl get pod -l app=test -o yaml | grep initContainers
# Output: No initContainers, no sidecar injection

Dependencies

none

Related Issues

none

✅ Contributor Checklist

  • I have tested these changes with kustomize. See Installation Prerequisites.
  • All commits are signed-off to satisfy the DCO check.
  • I have considered adding my company to the adopters page to support Kubeflow and help the community, since I expect help from the community for my issue (see 1. and 2.).

You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-platform.

@juliusvonkohout

Copy link
Copy Markdown
Member

Please rebase, there were a lot of test file changes.

@juliusvonkohout

juliusvonkohout commented Jun 3, 2025

Copy link
Copy Markdown
Member

Lets also use istio-install/overlays/legacy instead of standard. Or maybe even just "insecure"

Comment thread apps/kserve/README.md
Comment thread common/istio-1-26/README.md Outdated
Comment thread common/istio-1-26/README.md Outdated
Comment thread common/istio-1-26/README.md Outdated
Comment thread common/istio-1-26/README.md Outdated
@madmecodes madmecodes force-pushed the merge-istio-folders branch from 0c57cc3 to 5e4de14 Compare June 3, 2025 09:45
@madmecodes

madmecodes commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

@juliusvonkohout
The Problem i'm facing right now is on applying, common/istio-1-26/istio-install/overlays/insecure, istio-cni daemon set still present its not getting deleted, looking into this.

PS: solved, pushing the code

@madmecodes

Copy link
Copy Markdown
Contributor Author

@juliusvonkohout Open for feedback

@madmecodes

Copy link
Copy Markdown
Contributor Author

Kserve failing tests were fixed in this PR #3148

Comment thread common/istio-1-26/README.md Outdated
Comment thread common/istio-1-26/README.md Outdated

**Important**: You must delete the current installation before switching to avoid resource conflicts.

### Switch from CNI to Non-CNI

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can leave out the switch documentation and we shall always apply istio with the oauth2-proxy configuration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment thread tests/trivy_scan.py
@juliusvonkohout

Copy link
Copy Markdown
Member

Please also remove the version number and make sure that a diff -u of the manifests for istio yields exactly the same

@juliusvonkohout

Copy link
Copy Markdown
Member

Kserve failing tests were fixed in this PR #3148

Are you really sure. The other PR also has to be reworked.

@juliusvonkohout

Copy link
Copy Markdown
Member

And let's remove the version number 1-26 from the folder path and update the script in /scripts accordingly. It should be an in-place upgrade from now on.

@madmecodes

madmecodes commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

@juliusvonkohout There was a small issue in the Kserve test, this passed success: #3155

Screenshot 2025-06-03 at 9 25 31 PM

shall i do the changes in this PR only or rebase?

@madmecodes madmecodes force-pushed the merge-istio-folders branch from 05f5b8e to ff5b75b Compare June 3, 2025 16:31
@madmecodes

Copy link
Copy Markdown
Contributor Author

rebased

Comment thread common/istio/README.md Outdated
Comment thread common/istio/README.md
Comment thread common/istio/istio-install/overlays/insecure/install-insecure.yaml Outdated
Comment thread common/istio/istio-install/overlays/insecure/kustomization.yaml Outdated

@juliusvonkohout juliusvonkohout Jun 3, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please redo this from scratch. It should only patch out the CNI enablement in the sidecar. Nothing else. More than 30 line sin total is probably not the right way. Especially more than 20.000 lines. We dont want to maintain this many lines.

@juliusvonkohout juliusvonkohout Jun 9, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did you do with

        - name: EXTERNAL_ISTIOD
          value: "false"
        - name: PILOT_ENABLE_CROSS_CLUSTER_WORKLOAD_ENTRY
          value: "true"
        - name: PILOT_ENABLE_WORKLOAD_ENTRY_AUTOREGISTRATION
          value: "true"
        - name: PILOT_SKIP_VALIDATE_TRUST_DOMAIN
          value: "true"

Why did you add this? WHy is it needed to disable istio-cni?

where is the simple inline delete patch for the daemonset?

juliusvonkohout and others added 3 commits June 10, 2025 12:43
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
…ctor configmap to disable CNI as fallback option.

Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
@madmecodes madmecodes force-pushed the merge-istio-folders branch from a09ce5b to b7af4f8 Compare June 10, 2025 13:50
madmecodes and others added 8 commits June 13, 2025 19:09
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
…g because the istio-sidecar-injector ConfigMap had incomplete values structure, causing template unmarshalling errors

Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
@juliusvonkohout

Copy link
Copy Markdown
Member

Please test locally execttly the GHA with a fresh cluster, nothing else. I assume that for the insecure version you have to patch the kuberflwo, knative-serving, auth etc. namepsace PSS labels from enforce to warning in the insecure overlay.

Signed-off-by: madmecodes <ayushguptadev1@gmail.com>

@juliusvonkohout juliusvonkohout Jun 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kubeflow, oauth2-proxy. auth, knative-serving, the user namespace etc. are missing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding them in a new commit

Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
@madmecodes

Copy link
Copy Markdown
Contributor Author

@juliusvonkohout Kserve and Istio tests have passed now

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? i think the profile controller creates it by default


- name: Apply Pod Security Standards baseline levels
if: matrix.istio-mode == 'cni'
run: ./tests/PSS_baseline_enable.sh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- name: Apply Pod Security Standards baseline levels
  if: matrix.istio-mode == 'cni'
  run: ./tests/PSS_baseline_enable.sh can be removed. Most of them are PSS restricted by default now.

@juliusvonkohout

juliusvonkohout commented Jun 18, 2025

Copy link
Copy Markdown
Member

Thank you.

Please check https://github.com/kubeflow/manifests/pull/3153/files#r2154276621 in a follow up PR
/lgtm
/approve

@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juliusvonkohout

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 9cd0cfb into kubeflow:master Jun 18, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants