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.

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

Bump istio installation #1331

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Conversation

aerosouund
Copy link
Member

What this PR does / why we need it:

Bump Istio to the latest stable release

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1324

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Nov 26, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign davidvossel for approval. For more information see the Kubernetes Code Review Process.

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

@aerosouund
Copy link
Member Author

/test check-provision-k8s-1.29

@mhenriks
Copy link
Member

mhenriks commented Dec 3, 2024

@aerosouund thanks for taking a crack at this! I am very keen to get this in. Do you need any help? The current version we are using is very old, and is not compatible with kubevirt/kubevirt#13422. The istio sidecar injection code is somehow stealing the runStrategy: Always bit from virt-tail. And since the istio container is not meant to always run, the virt-luancher pod never gets to running state.

cc @EdDev

@aerosouund
Copy link
Member Author

@mhenriks
Thanks alot, I am facing a small challenge where after installation, checking the IstioOperator resource says that such a resource doesn't exist.
I am suspecting its because many things changed between the version we had and the version i am introducing 1.15 >> 1.24
I just need to check whats going on, but likely its not a big deal. I will ping you if i need any assistance!

@aerosouund
Copy link
Member Author

On inspecting the cluster post running istioctl install -y (the operator init has been deprecated) it seems it the IstioOperator resource is no longer used

[vagrant@node01 ~]$ sudo kubectl --kubeconfig=/etc/kubernetes/admin.conf api-resources | grep istio
wasmplugins                                      extensions.istio.io/v1alpha1      true         WasmPlugin
destinationrules                    dr           networking.istio.io/v1            true         DestinationRule
envoyfilters                                     networking.istio.io/v1alpha3      true         EnvoyFilter
gateways                            gw           networking.istio.io/v1            true         Gateway
proxyconfigs                                     networking.istio.io/v1beta1       true         ProxyConfig
serviceentries                      se           networking.istio.io/v1            true         ServiceEntry
sidecars                                         networking.istio.io/v1            true         Sidecar
virtualservices                     vs           networking.istio.io/v1            true         VirtualService
workloadentries                     we           networking.istio.io/v1            true         WorkloadEntry
workloadgroups                      wg           networking.istio.io/v1            true         WorkloadGroup
authorizationpolicies               ap           security.istio.io/v1              true         AuthorizationPolicy
peerauthentications                 pa           security.istio.io/v1              true         PeerAuthentication
requestauthentications              ra           security.istio.io/v1              true         RequestAuthentication
telemetries                         telemetry    telemetry.istio.io/v1             true         Telemetry

With that said, the yaml files we use to install the operator will no longer work (istio-operator.cr.yaml & istio-operator-with-cnao.cr.yaml)
This signifies a huge migration from what we have, previously what you would have in the cluster is

[vagrant@node01 ~]$ sudo kubectl --kubeconfig=/etc/kubernetes/admin.conf get pods -A | grep istio
istio-operator   istio-operator-6c4fc4d784-2qqff            1/1     Running   0          2m50s
istio-system     istio-egressgateway-79c995f7cb-w4ppg       1/1     Running   0          88s
istio-system     istio-ingressgateway-775fdbc456-qq5cn      1/1     Running   0          88s
istio-system     istiod-5857496459-cnz5t                    1/1     Running   0          92s
kube-system      istio-cni-node-l99td                       1/1     Running   0          88s

After the upgrade, you only get

[vagrant@node01 ~]$ sudo kubectl --kubeconfig=/etc/kubernetes/admin.conf get pods -A | grep istio
istio-system   istio-ingressgateway-5f9df778cc-bl9sw      1/1     Running   0          33m
istio-system   istiod-69d6bb74c-z6fqk                     1/1     Running   0          34m

So we need to know how to get the same as what we had before in the previous version using 1.24. I might need help from the network team on this

cc: @oshoval @EdDev

@aerosouund
Copy link
Member Author

Alternatively, we may not jump to 1.24. Maybe a lesser version that still behaves similar to 1.15 and has the things you want supported.
Needs a discussion for sure

@oshoval
Copy link
Contributor

oshoval commented Dec 5, 2024

I am not familiar at all with Istio tbh, and currently on few other tasks,
if discussion will be still needed, i will try once i can, thanks

@aerosouund
Copy link
Member Author

@oshoval
based on what changed across the deployments three questions need answered:

  • is the egressgateway still needed or used ? has it's functionality been ported to any of the other components ?
  • is the istiocni now being installed separately from the operator ? is yes, how ?
  • would it be better to not skip to a very high version and upgrade to something intermediate? e.g: 1.18

@oshoval
Copy link
Contributor

oshoval commented Dec 5, 2024

the first two need deeper understanding that i dont have now sorry,
if the 3rd helps advancing, lets go for that first thing, wdyt?
knowing where it stopped working, can also assist in debug latest sometimes

@aerosouund
Copy link
Member Author

@mhenriks
whats the minimum compatible version that will contain the features you want ?

@mhenriks
Copy link
Member

mhenriks commented Dec 5, 2024

@mhenriks whats the minimum compatible version that will contain the features you want ?

1.20 is the oldest version that supports 1.29 (SideCar featuregate enabled) so that would be the minimum. But obviously latest would be best

https://istio.io/latest/docs/releases/supported-releases/#support-status-of-istio-releases

@ormergi
Copy link
Contributor

ormergi commented Dec 5, 2024

* is the egressgateway still needed or used ? has it's functionality been ported to any of the other components ?

It seem we dont use in in e2e tests, we do use ingress-gateway though Istio API
https://github.com/kubevirt/kubevirt/blob/94d03645710c55dd35cf752bf6709bfb333518ad/tests/network/vmi_istio.go#L640
https://istio.io/latest/docs/setup/additional-setup/gateway/#deploying-a-gateway

* is the istiocni now being installed separately from the operator ? is yes, how ?

From what I remember Istio operator account for deploying its CNI, we have no dedicated scripting for doing that.
Check out https://istio.io/latest/docs/setup/additional-setup/cni/#installing-the-cni-node-agent

* would it be better to not skip to a very high version and upgrade to something intermediate? e.g: 1.18

Please note kubevirt e2e tests relays on the sidecar injection functionally.

I suggest to test this PR on kubevirt/kubevirt on sig-network lane so we can see where it fails and realize what our options.

@aerosouund
Copy link
Member Author

@mhenriks @ormergi
Ok, based on this i will move the check for the IstioOperator resource (which is what's failing the tests here) and devise a way to install the CNI. Then we can run it against the sig network lane and see how it goes

@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2025
@aerosouund
Copy link
Member Author

@ormergi Thanks alot for your help on this

Istio 1.23 should work as expected, I suggest using this version.

Acknowledged, thanks.

This cant work for kubevirtci env because we do not create the istio-cni NAD in the default namespace, but at the test namespace.
In general it seems wrong because it means all pods in the cluster can connect to the mesh, becasuse all pods can connect to a NAD resied in default namespaces, no isolation allowed.

Not sure i quite follow through but in general i will test this on kubevirtCI and check if it produces the desired results

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2025
@brianmcarey
Copy link
Member

Hey @aerosouund , I was wondering what the status of this one is? Have you tried with istio 1.23?

@aerosouund
Copy link
Member Author

No
I will. The main problem is that while the VM starts i am still seeing failures on kubevirt istio tests. But i am not sure if the problem is local to my environment or there's an actual problem is in istio. I will rewrite to 1.23 shortly

@brianmcarey
Copy link
Member

on kubevirt istio tests. But i am not sure if the problem is local to my environment or there's an actual problem is in istio. I will rewrite to 1.23 shortly

I can try and help to get a provider published once the changes are in place and we can test it against the actual CI environment to see how it looks.

@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2025
Signed-off-by: aerosouund <[email protected]>
@kubevirt-bot
Copy link
Contributor

@aerosouund: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
check-provision-k8s-1.29 ac32724 link true /test check-provision-k8s-1.29
check-provision-k8s-1.31-s390x 8616f8e link false /test check-provision-k8s-1.31-s390x
check-provision-k8s-1.31 8616f8e link true /test check-provision-k8s-1.31

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. I understand the commands that are listed here.

@brianmcarey
Copy link
Member

brianmcarey commented Feb 5, 2025

on kubevirt istio tests. But i am not sure if the problem is local to my environment or there's an actual problem is in istio. I will rewrite to 1.23 shortly

I can try and help to get a provider published once the changes are in place and we can test it against the actual CI environment to see how it looks.

I published a provider for this and ran the network e2e suite against it - it looks like the istio tests are still failing against this updated version. Attaching the artifacts from the run:
artifacts20250204.tar.gz

The images should be available from my quay repo
quay.io/bcarey/gocli:20250204-istiobump
quay.io/bcarey/k8s-1.31:20250204-istiobump

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2025
@kubevirt-bot
Copy link
Contributor

PR needs rebase.

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.

@ormergi
Copy link
Contributor

ormergi commented Feb 9, 2025

on kubevirt istio tests. But i am not sure if the problem is local to my environment or there's an actual problem is in istio. I will rewrite to 1.23 shortly

I can try and help to get a provider published once the changes are in place and we can test it against the actual CI environment to see how it looks.

I published a provider for this and ran the network e2e suite against it - it looks like the istio tests are still failing against this updated version. Attaching the artifacts from the run: artifacts20250204.tar.gz

The images should be available from my quay repo quay.io/bcarey/gocli:20250204-istiobump quay.io/bcarey/k8s-1.31:20250204-istiobump

It seems tests that verifies Isito & passt binding plugin are failing, tests that verifies masquerade binding pass.
Tests that specify port forward 22 explicitly in the VMI spec fails (7/8) because the VM didn't become ready, due to the following error coming form passt process in the VMI virt-launcher pod:

<failure type="Failure">tests/network/vmi_istio.go:152 Unexpected Warning event received: testvmi-2twdr,c3947566-91a2-415c-95f7-83cd5367ce16: server error. command SyncVMI failed: "LibvirtError(Code=1, Domain=0, Message='internal error: Child process (passt --one-off --socket /var/run/libvirt/qemu/run/passt/1-kubevirt-test-defaul-ua-default.socket --pid /var/run/libvirt/qemu/run/passt/1-kubevirt-test-defaul-ua-default-passt.pid --interface eth0 --log-file /var/run/kubevirt/passt.log --tcp-ports '~15000,~15001,~15004,~15006,~15008,~15009,~15020,~15021,~15053,~15090,1500,1501,22') unexpected exit status 1: Failed to bind port 22 (Permission denied) for option '-t ~15000,~15001,~15004,~15006,~15008,~15009,~15020,~15021,~15053,~15090,1500,1501,22', exiting\n')" Expected <string>: Warning not to equal <string>: Warning tests/watcher/watcher.go:195</failure>

Last time I ran these tests with newer istio they passed, I never saw such failures before tbh.
Its hard to tell where it falls, given recent changes in kubevirtci CRI-O un-freeze and CentOS bump.
@brianmcarey can you share the PR / failing prow job link? which lane was it?
Does it include kubevirt/kubevirt#13422 bits?

I will try to reproduce it with the images you mentioned and update.

ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing.

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [1].

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [2]

[1] istio/istio#54815
[2] kubevirt/kubevirtci#1331

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing [1].

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [2].

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [3]

[1] kubevirt/kubevirtci#1331 (comment)
[2] istio/istio#54815
[3] kubevirt#13832

Signed-off-by: Or Mergi <[email protected]>
@ormergi
Copy link
Contributor

ormergi commented Feb 12, 2025

According to #1331 (comment) attached artifacts we see istio+passt tests are failing due to bug in Istio.
I posted a PR to quarantine the failing tests, once we have a workaround or bug fix we can un-quarentine them.
PTAL kubevirt/kubevirt#13924

ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing [1].

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [2].
VM with passt binding plugin produce virt-laucher pod with Multus
network annotation in JSON form.

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [3]

[1] kubevirt/kubevirtci#1331 (comment)
[2] istio/istio#54820
[3] kubevirt#13832

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing [1].

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [2].
VM with passt binding plugin produce virt-laucher pod with Multus
network annotation in JSON form.

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [3]

[1] kubevirt/kubevirtci#1331 (comment)
[2] istio/istio#54820
[3] kubevirt#13832

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing [1].

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [2].
VM with passt binding plugin produce virt-laucher pod with Multus
network annotation in JSON form.

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [3]

[1] kubevirt/kubevirtci#1331 (comment)
[2] istio/istio#54820
[3] kubevirt#13832

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing [1].

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [2].
VM with passt binding plugin produce virt-laucher pod with Multus
network annotation in JSON form.

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [3]

[1] kubevirt/kubevirtci#1331 (comment)
[2] istio/istio#54820
[3] kubevirt#13832

Signed-off-by: Or Mergi <[email protected]>
ormergi added a commit to ormergi/kubevirt that referenced this pull request Feb 12, 2025
CI runs very old version of Istio that does not support new k8s
features and block development that utilize such features (e.g.: restart
policy for init containers).

The is an effort to upgrade Istio version CI use, but it fails due to
because some Istio+passt tests failing [1].

Turn out VMs who's virt-laucher pod have Multus network annotation in
JSON form doesn't work with newer version of Istio [2].
VM with passt binding plugin produce virt-laucher pod with Multus
network annotation in JSON form.

Quarantine istio+passt failing tests to unblock istio upgrade and
development.

Tracking issue for istio upgrade [3]

[1] kubevirt/kubevirtci#1331 (comment)
[2] kubevirt#13927
[3] kubevirt#13832

Signed-off-by: Or Mergi <[email protected]>
@ormergi
Copy link
Contributor

ormergi commented Feb 12, 2025

Hi, I managed to make istio 1.24.3 work with the following config:

apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
metadata:
  namespace: istio-system
  name: istio-operator
spec:
  profile: demo
  components:
    cni:
      enabled: true
      namespace: kube-system
      k8s:
        securityContext:
          seLinuxOptions:
            type: spc_t
  values:
    global:
      jwtPolicy: third-party-jwt
    cni:
      chained: false
      cniBinDir: /opt/cni/bin
      cniConfDir: /etc/cni/multus/net.d
      excludeNamespaces:
       - istio-system
       - kube-system
      logLevel: debug
    pilot:
      cni:
        enabled: true
        provider: "multus"
    sidecarInjectorWebhook:
      injectedAnnotations:
        k8s.v1.cni.cncf.io/networks: istio-cni

Missing part was setting Isito CNI pods with seLinuxOptions spc_t, following discussion at istio/istio#54815
@aerosouund @brianmcarey

@aerosouund
Copy link
Member Author

aerosouund commented Feb 12, 2025

@ormergi
Great work! Will push this to the branch ASAP. Do you prefer to stick with 1.23 or do we use 1.24?

@ormergi
Copy link
Contributor

ormergi commented Feb 12, 2025

. Do you prefer to stick with 1.23 or do we use 1.24?

I prefer newer version, but if its a trouble we can start with 1.23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/enhancement needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump Istio to the latest stable release
7 participants