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

Add Pod as an owner of a CiliumEndpoint and remove useless Delete #11195

Merged
merged 4 commits into from May 1, 2020

Conversation

aanm
Copy link
Member

@aanm aanm commented Apr 28, 2020

Remove Delete operation when starting up Cilium which removes the CiliumEndpoint in the k8s apiserver. This is unnecessary since CiliumEndpoint will be updated immediately afterwards.
As a CiliumEndpoint can't exist without a Pod, we should set the Pod as an Owner of a particular endpoint.
This will allow CiliumEndpoints from being automatically GCed by K8s once the Pod is gone.

Do not delete CiliumEndpoint on Cilium restart and add Pod as a Owner of a CiliumEndpoint

@aanm aanm added kind/enhancement This would improve or streamline existing functionality. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Apr 28, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 28, 2020
@aanm
Copy link
Member Author

aanm commented Apr 28, 2020

test-me-please

@coveralls
Copy link

coveralls commented Apr 28, 2020

Coverage Status

Coverage increased (+0.009%) to 44.61% when pulling 8c9bbad on pr/add-ownwer-to-ciliumendpoint into 2e957af on master.

@aanm aanm force-pushed the pr/add-ownwer-to-ciliumendpoint branch from 215aa3d to 60765fa Compare April 28, 2020 19:58
@aanm
Copy link
Member Author

aanm commented Apr 28, 2020

test-me-please

@aanm aanm marked this pull request as ready for review April 28, 2020 20:36
@aanm aanm requested a review from a team April 28, 2020 20:36
@aanm aanm requested a review from a team as a code owner April 28, 2020 20:36
@aanm aanm requested a review from a team April 28, 2020 20:36
daemon/cmd/endpoint.go Show resolved Hide resolved
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
During initialization, it is not required to delete the CEP as we will
overwrite its status regardless of the state stored in k8s.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm force-pushed the pr/add-ownwer-to-ciliumendpoint branch from 60765fa to 59a5141 Compare April 29, 2020 14:52
@aanm aanm requested a review from tgraf April 29, 2020 14:52
As a CiliumEndpoint can't exist without a Pod, we should set the Pod as
an Owner of a particular endpoint.

This will allow CiliumEndpoints from being automatically GCed by K8s
once the Pod is gone.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm force-pushed the pr/add-ownwer-to-ciliumendpoint branch from 59a5141 to 8c9bbad Compare April 29, 2020 14:53
@aanm
Copy link
Member Author

aanm commented Apr 29, 2020

test-me-please

@@ -130,34 +130,37 @@ func (epSync *EndpointSynchronizer) RunK8sCiliumEndpointSync(e *endpoint.Endpoin
return nil
}

scopedLog.Debug("Deleting CEP during an initialization")
err := ciliumClient.CiliumEndpoints(namespace).Delete(ctx, podName, meta_v1.DeleteOptions{})
Copy link
Member

Choose a reason for hiding this comment

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

I think this was done a long time ago when we changed the format of the CIliumEndpoint at the very beginning for this feature and this allowed to up and downgrade.

@@ -211,6 +212,9 @@ type Endpoint struct {
// K8sNamespace is the Kubernetes namespace of the endpoint
K8sNamespace string

// pod
Copy link
Member

Choose a reason for hiding this comment

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

💎

@aanm aanm merged commit 08dc8ca into master May 1, 2020
1.8.0 automation moved this from In progress to Merged May 1, 2020
@aanm aanm deleted the pr/add-ownwer-to-ciliumendpoint branch May 1, 2020 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This would improve or streamline existing functionality. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

3 participants