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

Conflict with istio-cni-node when cilium is the main CNI plugin #26760

Closed
2 tasks done
demikl opened this issue Jul 11, 2023 · 8 comments · Fixed by #26773
Closed
2 tasks done

Conflict with istio-cni-node when cilium is the main CNI plugin #26760

demikl opened this issue Jul 11, 2023 · 8 comments · Fixed by #26773
Assignees
Labels
affects/v1.14 This issue affects v1.14 branch area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. sig/agent Cilium agent related.

Comments

@demikl
Copy link

demikl commented Jul 11, 2023

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

Cilium overwrites the configuration that istio-cni wants to append to the cilium cni config file.

Cilium Version

v1.14.0-rc.0

Kernel Version

Linux ip-100-89-14-191.eu-west-3.compute.internal 5.10.184-175.731.amzn2.x86_64 #1 SMP Tue Jun 27 21:48:55 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Kubernetes Version

Server Version: version.Info{Major:"1", Minor:"24+", GitVersion:"v1.24.14-eks-c12679a", GitCommit:"05d192f0de17608d98e17761ad3cffa9a6407f2f", GitTreeState:"clean", BuildDate:"2023-05-22T23:41:27Z", GoVersion:"go1.19.9", Compiler:"gc", Platform:"linux/amd64"}

Sysdump

No response

Relevant log output

2023-07-11T08:19:29.414585Z    info    install    CNI config file /host/etc/cni/net.d/05-cilium.conflist exists. Proceeding.
2023-07-11T08:19:29.415159Z    info    install    Created CNI config /host/etc/cni/net.d/05-cilium.conflist
2023-07-11T08:19:29.415192Z    info    install    CNI configuration and binaries reinstalled.
2023-07-11T08:19:29.430298Z    info    install    Detect changes to the CNI configuration and binaries, attempt reinstalling...

Anything else?

istio-cni appends its configuration to /etc/cni/net.d/05-cilium.conflist:

{
  "cniVersion": "0.3.1",
  "name": "cilium",
  "plugins": [
    {
      "enable-debug": false,
      "log-file": "/var/run/cilium/cilium-cni.log",
      "type": "cilium-cni"
    },
    {
      "kubernetes": {
        "cni_bin_dir": "/opt/cni/bin",
        "exclude_namespaces": [
          "istio-system",
          "kube-system"
        ],
        "kubeconfig": "/etc/cni/net.d/ZZZ-istio-cni-kubeconfig"
      },
      "log_level": "info",
      "log_uds_address": "/var/run/istio-cni/log.sock",
      "name": "istio-cni",
      "type": "istio-cni"
    }
  ]
}

but cilium immediately reverts it back.

Using cni-exclusive: false does not change this behavior.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@demikl demikl added kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. needs/triage This issue requires triaging to establish severity and next steps. labels Jul 11, 2023
@demikl
Copy link
Author

demikl commented Jul 11, 2023

cilium sysdump attached
cilium-sysdump-20230711-152150.zip

@demikl
Copy link
Author

demikl commented Jul 11, 2023

As far as I can understand, the behavior has changed recently through #24389
The startup behavior takes into account the cni-exclusive flag, but the goroutine that constantly watches any changes in the CNI configuration directory is not using this flag.

@julianwiedmann julianwiedmann added area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. sig/agent Cilium agent related. labels Jul 11, 2023
@julianwiedmann
Copy link
Member

@squeed sounds like right down your alley - any pointers? :)

@squeed squeed added the affects/v1.14 This issue affects v1.14 branch label Jul 11, 2023
@squeed
Copy link
Contributor

squeed commented Jul 11, 2023

Interesting; it looks like both Cilium and Istio both want to own this configuration file.

My proposal would be to fix this by adding a new Helm value, cni.writeOnce, which disables the file watcher and, instead, writes once and exits.

@squeed
Copy link
Contributor

squeed commented Jul 12, 2023

Aha, that sounds like a use-case the code failed to anticipate.

I think it would be reasonable to disable the CNI file watcher if cni.exclusive is set to false. I'll submit a change for that.

It will get backported, but might not make v1.14.0.

@squeed squeed removed the needs/triage This issue requires triaging to establish severity and next steps. label Jul 12, 2023
@squeed squeed self-assigned this Jul 12, 2023
@squeed
Copy link
Contributor

squeed commented Jul 12, 2023

Filed #26773 to disable the file-watcher when cni.exclusive is false.

@demikl
Copy link
Author

demikl commented Jul 13, 2023

Thanks for handling this. Do you have a build that I can test ?

@squeed
Copy link
Contributor

squeed commented Jul 13, 2023

@demikl it's not exactly production grade, but you can pull the CI image built in this action from quay.io/cilium/cilium-ci:e1d97737c2836c42aae7b30ca8b152622ae7e0c8@sha256:0d94072b57aa04efd5808239bd63021446045b404dc39c607d04bf5b7dae8f12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.14 This issue affects v1.14 branch area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. sig/agent Cilium agent related.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants