cilium / cilium

eBPF-based Networking, Security, and Observability
https://cilium.io
Apache License 2.0
20.17k stars 2.96k forks source link

OwnerReferences removal by Cilium Operator #32339

Closed IvanProdaiko94 closed 4 months ago

IvanProdaiko94 commented 6 months ago

Is there an existing issue for this?

What happened?

All of the commands below are executed on top of the cluster with cilium-operator running.

Creating a POD:

kubectl apply -f ./tmp.yaml

apiVersion: v1
kind: Pod
metadata:
  name: nginx-pod
  namespace: test
  labels:
    app: nginx
spec:
  containers:
    - name: nginx-container
      image: nginx:latest
      ports:
        - containerPort: 80

Creating a Service (knowing UID of the pod):

kubectl apply -f ./tmp_service.yaml

apiVersion: v1
kind: Service
metadata:
  name: nginx-loadbalancer-service
  namespace: test
  ownerReferences:
    - apiVersion: v1
      blockOwnerDeletion: true
      controller: true
      kind: Pod
      name: nginx-pod
      uid: ${uid}
spec:
  selector:
    app: nginx
  type: LoadBalancer
  ports:
  - name: http
    port: 80
    targetPort: 8080
    protocol: TCP

Execute:

kubectl get src -n test -o yaml

One will end up with no ownerReferences in place.

If we will do the same thing without cilium-operator running the ownerReferences would be there.

Cilium Version

v.1.13.3

Kernel Version

Linux node-u2bnattm.data.stable.pndrs.de 4.18.0-425.3.1.el8.x86_64 #1 SMP Fri Sep 30 11:45:06 EDT 2022 x86_64 x86_64 x86_64 GNU/Linux

Kubernetes Version

kubectl version

WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.15+rke2r1", GitCommit:"2c67202dc0bb96a7a837cbfb8d72e1f34dfc2808", GitTreeState:"clean", BuildDate:"2023-06-14T21:17:38Z", GoVersion:"go1.19.10 X:boringcrypto", Compiler:"gc", Platform:"linux/amd64"}

Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.15+rke2r1", GitCommit:"2c67202dc0bb96a7a837cbfb8d72e1f34dfc2808", GitTreeState:"clean", BuildDate:"2023-06-14T21:17:38Z", GoVersion:"go1.19.10 X:boringcrypto", Compiler:"gc", Platform:"linux/amd64"}

Regression

No response

Sysdump

No response

Relevant log output

No response

Anything else?

No response

Cilium Users Document

Code of Conduct

youngnick commented 6 months ago

Thanks for this issue @IvanProdaiko94, this is most likely associated with either the code that handles L2 announcements (and so handles Loadbalancer services) or the Ingress and Gateway code being aggressive in trimming ownerReferences. I've marked it for someone to take a look at.

chaunceyjiang commented 6 months ago

@IvanProdaiko94 Have you tried this step in the latest version of Cilium? I seem unable to reproduce this issue in the latest version.

IvanProdaiko94 commented 6 months ago

@chaunceyjiang it seems you're right

IvanProdaiko94 commented 6 months ago

@chaunceyjiang @youngnick So, folks. I checked 1.13.3, 1.14.10, 1.15.4 and all of them remove ownerReferences. With 1.16.0-pre.2 running on cluster ownerReferences are not removed, but bgp is not assigning the IP for LoadBalancer. I assume, that if only the bgp tried to assign the IP, it will do a removal as well

chaunceyjiang commented 6 months ago

but bgp is not assigning the IP for LoadBalancer. I assume, that if only the bgp tried to assign the IP, it will do a removal as well

I will try to reproduce it locally.

chaunceyjiang commented 5 months ago

I checked 1.13.3, 1.14.10, 1.15.4 and all of them remove ownerReferences.

It seems like I also can't reproduce it.

root@node1:~/cilium# kubectl get svc nginx-loadbalancer-service -oyaml
apiVersion: v1
kind: Service
metadata:
  creationTimestamp: "2024-05-30T08:12:13Z"
  labels:
    a: b
  name: nginx-loadbalancer-service
  namespace: default
  ownerReferences:
  - apiVersion: v1
    blockOwnerDeletion: true
    controller: true
    kind: Pod
    name: gpu-brun-695c587cdc-klq86
    uid: c45f48df-66b6-4300-b3f5-eda0c2a2e1a3
  resourceVersion: "51694462"
  uid: 21cde266-2c0a-4389-9641-bf34c2eed822
spec:
  allocateLoadBalancerNodePorts: true
  clusterIP: 10.96.144.66
  clusterIPs:
  - 10.96.144.66
  externalTrafficPolicy: Cluster
  internalTrafficPolicy: Cluster
  ipFamilies:
  - IPv4
  ipFamilyPolicy: SingleStack
  ports:
  - name: http
    nodePort: 31767
    port: 80
    protocol: TCP
    targetPort: 8080
  selector:
    app: nginx
  sessionAffinity: None
  type: LoadBalancer
status:
  conditions:
  - lastTransitionTime: "2024-05-30T08:32:19Z"
    message: ""
    reason: satisfied
    status: "True"
    type: cilium.io/IPAMRequestSatisfied
  loadBalancer:
    ingress:
    - ip: 172.30.126.134
PhilipSchmid commented 5 months ago

The MetalLB-based BGP implementation (deprecated) seems to always remove the ownerReferences. The new Cilium go-bgp-based BGP Control Plance doesn't. Solution/workaround: Use the new BGP Control Plane.

IMO, this issue could be closed.

aanm commented 4 months ago

Closing as suggested in https://github.com/cilium/cilium/issues/32339#issuecomment-2139306920