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

Use an init container for CNI when in k8s api datastore mode #2231

Merged

Conversation

caseydavenport
Copy link
Member

@caseydavenport caseydavenport commented Oct 13, 2018

Description

I'd like to do this for etcd as well, but it needs more thought first. There's some code which runs and updates certs if they change. I'm not sure if anyone uses that code, so maybe we can just drop it.

I say we do it for kdd anyway, since it's an easy win.

  • One fewer docker containers per node required (which takes up resources).
  • kubectl logs no longer requires specifying container name.
  • simpler kubectl describe output, since it's only for one container.

Todos

  • Tests
  • Documentation
  • Release note

Release Note

The CNI plugin is now installed using a Kubernetes init container rather than a long-lived sidecar.

@caseydavenport caseydavenport requested a review from a team as a code owner October 13, 2018 21:38
@caseydavenport caseydavenport added the release-note-required Change has user-facing impact (no matter how small) label Oct 13, 2018
@caseydavenport caseydavenport changed the title WIP: Use an init container for CNI when in k8s api datastore mode Use an init container for CNI when in k8s api datastore mode Oct 13, 2018
@fasaxc
Copy link
Member

fasaxc commented Oct 15, 2018

Any way we can avoid the duplication in the template? I see the value in simplifying for the user and reducing our resource use but if the template gets duplicated we're more likely to introduce bugs in maintenance.

@fasaxc
Copy link
Member

fasaxc commented Oct 15, 2018

Think I'd prefer making such a change at the start of a release cycle; just to give us time to spot any unexpected quirk of initContainers.

@caseydavenport
Copy link
Member Author

Think I'd prefer making such a change at the start of a release cycle; just to give us time to spot any unexpected quirk of initContainers.

Yep, SGTM. Master is now v3.4 dev, so we should be OK now.

Any way we can avoid the duplication in the template? I see the value in simplifying for the user and reducing our resource use but if the template gets duplicated we're more likely to introduce bugs in maintenance.

Hm, I'll think about it. There may be a way. Honestly, I'd like to move etcd over to this approach as well anyway, so maybe we should just do that.

@caseydavenport caseydavenport added this to the Calico v3.4.0 milestone Oct 15, 2018
@caseydavenport caseydavenport merged commit 4925f65 into projectcalico:master Oct 16, 2018
@caseydavenport caseydavenport deleted the init-container-kdd branch October 16, 2018 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-required Change has user-facing impact (no matter how small)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants