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

SchedulerPredicates: pods with same hostPort but different hostIP and protocol #14287

Open
dghubble opened this issue Dec 4, 2020 · 29 comments
Assignees
Labels
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. pinned These issues are not marked stale by our issue bot. priority/high This is considered vital to an upcoming release. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.

Comments

@dghubble
Copy link
Contributor

dghubble commented Dec 4, 2020

Bug report

Some early testing of Kubernetes v1.20.0-rc.0, found one conformance failure with Cilium that's on the horizon. Filing this as an initial report.

[Fail] [sig-scheduling] SchedulerPredicates [Serial] [It] validates that there is no conflict between pods with same hostPort but different hostIP and protocol [Conformance] 
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes/runner.go:113

Full Logs

The test failure was repeatable. The test relates to pods exposing host ports. When using Calico (distro default) the same test did pass.

General Information

KVStore:                Ok   Disabled
Kubernetes:             Ok   1.20+ (v1.20.0-rc.0) [linux/amd64]
Kubernetes APIs:        ["cilium/v2::CiliumClusterwideNetworkPolicy", "cilium/v2::CiliumEndpoint", "cilium/v2::CiliumNetworkPolicy", "cilium/v2::CiliumNode", "core/v1::Namespace", "core/v1::Node", "core/v1::Pods", "core/v1::Service", "discovery/v1beta1::EndpointSlice", "networking.k8s.io/v1::NetworkPolicy"]
KubeProxyReplacement:   Probe   [ens5 (Direct Routing)]
Cilium:                 Ok      OK
NodeMonitor:            Disabled
Cilium health daemon:   Ok   
IPAM:                   IPv4: 4/255 allocated from 10.2.0.0/24, 
BandwidthManager:       Disabled
Host Routing:           Legacy
Masquerading:           BPF   [ens5]   10.2.0.0/24
Controller Status:      25/25 healthy
Proxy Status:           OK, ip 10.2.0.53, 0 redirects active on ports 10000-20000
Hubble:                 Disabled
Cluster health:         3/3 reachable   (2020-12-04T20:59:20Z)

How to reproduce the issue

A cluster can be provisioned and tested via CNCF docs if someone is inclined. Set the ref to master to get Kubernetes v1.20.x candidates and set networking to cilium. Abbreviated:

module "cluster-for-conformance" {
  source = "git::https://github.com/poseidon/typhoon//aws/fedora-coreos/kubernetes?ref=e77dd6ecd4380e542320638e34bfa9d07fe96bf1"
  ...
  # override default calico
  networking = "cilium"
  ...
}
@dghubble dghubble added the kind/bug This is a bug in the Cilium logic. label Dec 4, 2020
@aditighag
Copy link
Member

From the logs -

STEP: Trying to create a pod(pod1) with hostport 54321 and hostIP 127.0.0.1 and expect scheduled
STEP: Trying to create another pod(pod2) with hostport 54321 but hostIP 10.0.29.34 on the node which pod1 resides and expect scheduled
STEP: Trying to create a third pod(pod3) with hostport 54321, hostIP 10.0.29.34 but use UDP protocol on the node which pod2 resides
STEP: checking connectivity from pod e2e-host-exec to serverIP: 127.0.0.1, port: 54321

I don't think we support loopback address as a hostIP - https://github.com/cilium/cilium/blob/master/pkg/k8s/watchers/pod.go#L517

You should see this warning message in the cilium logs -
The requested loopback address for hostIP (%s) is not supported. Ignoring

Connectivity to the other hostIP should succeed.

\cc @borkmann

@aanm
Copy link
Member

aanm commented Dec 4, 2020

Also just to add to what Aditi said, the difference of protocols will likely be fixed by #9207

@aditighag
Copy link
Member

Also just to add to what Aditi said, the difference of protocols will likely be fixed by #9207

Yes, connectivity should work for both the protocols, though.

@stale
Copy link

stale bot commented Feb 22, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Feb 22, 2021
@dghubble
Copy link
Contributor Author

Kubernetes v1.20 conformance still fails with Cilium last I checked.

@stale stale bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Feb 22, 2021
@pchaigno pchaigno added 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 Feb 22, 2021
@aditighag
Copy link
Member

aditighag commented Feb 23, 2021

@dghubble Per the last comments -

There are 2 issues here :

  • loopback address isn't supported as a hostIP
    If you need this feature, can you mention what the use cases are?
  • Currently, we don't differentiate between L4 protocols (TCP, UDP)
    Differentiate between UDP and TCP services #9207 is still pending.

@brandond
Copy link

brandond commented May 1, 2021

CNI plugins should pass all conformance tests, regardless of whether or not a use case can be presented. This is blocking us from certifying Cilium for use with Rancher/RKE2.

@vadorovsky vadorovsky self-assigned this May 3, 2021
@dghubble
Copy link
Contributor Author

dghubble commented May 5, 2021

Yeah, this also precludes Typhoon making Cilium the default. As contrived as some of the Kubernetes conformance tests are the conformance matters to some users.

@dghubble
Copy link
Contributor Author

I tested with Cilium v1.10.0 on Kubernetes v1.21.1 and this was the one conformance failure.

@joestringer
Copy link
Member

Hi folks, just as a heads up one way to get this conformance test to pass is to run Cilium chaining on top of portmap, like I've done in PR #17048.

@dghubble
Copy link
Contributor Author

Chaining mode isn't supposed to be needed anymore, I don't see this as a config we'd go back to as a workaround and it had other issues in my testing.

@joestringer
Copy link
Member

For what it's worth, I briefly looked into the two issues that @aditighag mentioned above with the kube-proxy replacement implementation of this feature, notes below. I also notice that the Kubernetes best practices basically say "don't use hostport if at all possible" which is perhaps indicative of how tricky it can be to implement safely.

  • Supporting 127.0.0.1 is theoretically simple but comes with security concerns - in the past there have been upstream kubernetes bugs where exposing a service on 127.0.0.1 can unintentionally also expose the service to the cluster (See CVE-2020-8558). As far as I can tell, the conformance test doesn't intentionally specify that hostports must be able to be exposed on 127.0.0.1, this is just a convenient way to test the goal (below). I couldn't find any discussion in upstream Kubernetes PRs around this constraint.
  • The goal of the conformance test is to check that different services could be running on the same port but with different protocols (eg UDP vs. TCP). This aspect is not yet supported in KPR mainly because changing the service format can impact upgrade. @jibi has written up a summary of these concerns recently on the related issue.

I think that the above problems are solvable but they just require careful thought to mitigate the security and upgrade concerns.

@vadorovsky vadorovsky removed their assignment Dec 17, 2021
@aanm aanm added the sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. label Jan 6, 2022
@borkmann borkmann added the priority/high This is considered vital to an upcoming release. label Jan 21, 2022
@github-actions
Copy link

github-actions bot commented Jul 9, 2022

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 9, 2022
@dghubble
Copy link
Contributor Author

dghubble commented Jul 9, 2022

Pretty sure this conformance test will still fail with Kubernetes v1.24.2

@JBOClara
Copy link

With k8s v1.22.12 and cilium 1.12

sonobuoy results $results
Plugin: e2e
Status: failed
Total: 6442
Passed: 345
Failed: 1
Skipped: 6096

Failed tests:
[sig-network] HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]

Plugin: systemd-logs
Status: passed
Total: 6
Passed: 6
Failed: 0
Skipped: 0

@aspsk
Copy link
Contributor

aspsk commented Jul 22, 2022

With k8s v1.22.12 and cilium 1.12
[...]
Failed tests:
[sig-network] HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]

No surprises. As was mentioned above, this test will fail until the following two features are not supported:

This is work in progress.

@nathanjsweet
Copy link
Member

This test is passing now according to #21060

@dghubble
Copy link
Contributor Author

dghubble commented Apr 2, 2023

With Cilium v1.13.1, I still see this conformance test failing (with Kubernetes/Typhoon v1.26.3, manifests)

@ShiroDN
Copy link

ShiroDN commented Apr 21, 2023

I just tested cilium 1.13.1, including patch #21366 with k3s and kubeadm on ubuntu 22.04 and debian 11 (kubernetes 1.26.3), and test "HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol" is still failing for me.

EDIT: Sorry, I overlooked a post from aspsk where it is mentioned that we need to wait for #9207 to be implemented too.

@ShiroDN
Copy link

ShiroDN commented Apr 21, 2023

It's interesting that for example digitalocean managed kubernetes which uses cilium too all certified-conformance tests are passing. It seems they are using official cilium images, so what's the difference there?

@nathanjsweet
Copy link
Member

@ShiroDN you do have to configure Cilium in a particular way to get conformance tests passing. See kubernetes-test.sh for more details.

@tnorlin
Copy link
Contributor

tnorlin commented Apr 24, 2023

@nathanjsweet @aanm That link was great insight, but to me it looks like the kubernetes-test.sh is for passing with kube-proxy based cluster only? I've seen passing of conformance test in kube-proxy mode but #9207 is still open which was a blocker for strict / eBPF mode.

I tried an adaption of the script like below (ansible):

    name: cilium
    chart_ref: cilium/cilium
    #chart_version: "{{ cilium_version }}"
    chart_version: 1.13.2
    release_namespace: kube-system
    update_repo_cache: yes
    kubeconfig: /etc/kubernetes/admin.conf
    values:
      k8sServiceHost: "{{ cilium_service_host }}"
      k8sServicePort: "{{ cilium_service_port }}"
      debug:
        enabled: true
      k8s:
        requireIPv4PodCIDR: true
      pprof:
        enabled: true
      logSystemLoad: true
      bpf:
        preallocateMaps: true
      etcd:
        leaseTTL: "30s"
      ipv4:
        enabled: true
      ipv6:
        enabled: true
      identityChangeGracePeriod: "0s"
      cni:
        chainingMode: "portmap"
      sessionAffinity: true
      hubble:
        enabled: true
        metrics:
          serviceMonitor:
            enabled: true
          enableOpenMetrics: true
          enabled:
            - dns
          dashboards:
            enabled: true
            namespace: monitoring
            annotations:
              grafana_folder: "Cilium"
        relay:
          enabled: true
          prometheus:
            enabled: true
            serviceMonitor:
              enabled: true
        ui:
          enabled: true
      prometheus:
        enabled: true
        serviceMonitor:
          enabled: true
      operator:
        prometheus:
          enabled: true
          serviceMonitor:
            enabled: true

This had me the following result:

Summarizing 2 Failures:
  [FAIL] [sig-network] Services [It] should respect internalTrafficPolicy=Local Pod and Node, to Pod (hostNetwork: true) [Feature:ServiceInternalTrafficPolicy]
  test/e2e/network/util.go:175
  [FAIL] [sig-network] Services [It] should respect internalTrafficPolicy=Local Pod (hostNetwork: true) to Pod [Feature:ServiceInternalTrafficPolicy]
  test/e2e/network/util.go:175

Ran 74 of 7066 Specs in 3990.116 seconds
FAIL! -- 72 Passed | 2 Failed | 0 Pending | 6992 Skipped
--- FAIL: TestE2E (3990.16s)
FAIL

the ~/bin/sonobuoy run --mode=certified-conformance fails as well.

@ShiroDN
Copy link

ShiroDN commented Apr 25, 2023

@nathanjsweet Thanks for the link. Now, I managed to pass conformance test as well.
So basically, there is a need to install portmap cni-plugin to all hosts and set these flags to configmap:

cni-chaining-mode: portmap
enable-session-affinity: 'true'

@camaeel
Copy link

camaeel commented Jul 20, 2023

We are running v1.13.2, I enabled kube-proxy replacement in cilium, I also installed portmap cni, and set those:

cni-chaining-mode: portmap
enable-session-affinity: 'true'
kubeProxyReplacement: strict
enable-local-redirect-policy: "true"

but still kubernetes conformance test is failing (due to the fact that hostPort with UDP and TCP overwrite each other).

@ShiroDN
Copy link

ShiroDN commented Jul 20, 2023

@camaeel, I think it could be failing because of kubeProxyReplacement: strict. Strict mode actually disables the CNI chaining mode, if I'm not mistaken.

@camaeel
Copy link

camaeel commented Jul 20, 2023

@ShiroDN are you sure?
I couldn't find any docs about that, maybe except that in strict mode hostPort is handled by cilium. Maybe an option would be to have cilium kube-proxy replacement in partial mode, enable every other kube-proxy feature except hostPort, and enable portmap cni-chaining?

@ShiroDN
Copy link

ShiroDN commented Jul 20, 2023

@camaeel, yes, I am sure. I tested it by not setting the kubeProxyReplacement at all. The default is 'disabled', and it worked fine. All conformance tests were successfully completed. Here's something about it in the docs: https://docs.cilium.io/en/stable/network/kubernetes/kubeproxy-free/#container-hostport-support.

Cilium’s eBPF kube-proxy replacement also natively supports hostPort service mapping without having to use the Helm CNI chaining option of cni.chainingMode=portmap. By specifying kubeProxyReplacement=strict the native hostPort support is automatically enabled and therefore no further action is required...

@camaeel
Copy link

camaeel commented Jul 21, 2023

I tested workaround which seems to work:

  1. Install portmap binary on all nodes
  2. set (through helmChart's values.yaml):
    cni.chainingMode: portmap
    kubeProxyReplacement: partial
    socketLB:
      enabled: true
    nodePort:
      enabled: true
    externalIPs:
      enabled: true
    hostPort:
      enabled: false
    

Then Cilium's kube-proxy replacement works for everything except hostPorts, and hostPorts are handled by portmap CNI.

Are there any plans/schedule to implement final solution for this issue?

okozachenko1203 added a commit to vexxhost/magnum-cluster-api that referenced this issue Apr 10, 2024
Failure of `[sig-network] HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]` is expected until cilium/cilium#14287 is fixed
okozachenko1203 added a commit to vexxhost/magnum-cluster-api that referenced this issue Apr 10, 2024
Failure of `[sig-network] HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]` is expected until cilium/cilium#14287 is fixed
okozachenko1203 added a commit to vexxhost/magnum-cluster-api that referenced this issue Jun 18, 2024
Failure of `[sig-network] HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]` is expected until cilium/cilium#14287 is fixed
mnaser pushed a commit to vexxhost/magnum-cluster-api that referenced this issue Jun 20, 2024
* feat: support cilium cin

* fix lint error

* set version in manifest file

* use helm for cilium

* fix vendir

* add vendir in hack

* set cilium tag

* update makefile for vendir install

* remove cni assertion

* add validation step for network drivers

* fix tests

* add executable perm to vendir install script

* fix functional test

* resolve conflict

* fix typo

* add zuul ci jobs for cilium network driver

* enable sessionAffinity in cilium

For users who run with kube-proxy (i.e. with Cilium's kube-proxy replacement
disabled), the ClusterIP service loadbalancing when a request is sent from a pod
running in a non-host network namespace is still performed at the pod network
interface (until cilium/cilium#16197 is fixed).
For this case the session affinity support is disabled by default.

* fix flake8 errors

* ignore the below test failure until the upstream issue fixed

Failure of `[sig-network] HostPort validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]` is expected until cilium/cilium#14287 is fixed

* use portmap chain mode

* fix lint error

---------

Co-authored-by: okozachenko1203 <okozachenko1203@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. pinned These issues are not marked stale by our issue bot. priority/high This is considered vital to an upcoming release. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

No branches or pull requests