node/sdn: make /var/lib/cni persistent to ensure IPAM allocations stick around across node restart #13231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ck around across node restart With the move to a CNI plugin, docker no longer handles IPAM, but CNI does through openshift-sdn's usage of the 'host-local' CNI IPAM plugin. That plugin stores IPAM allocations under /var/lib/cni/. If the node container gets restarted, without presreving /var/lib/cni, the IPs currently allocated to running pods get lost and on restart, openshift-sdn may allocate those IPs to new pods causing duplicate allocations. This never happened with docker because it has its own persistent IPAM store that does not get removed when docker restarts. Also because (historically) when docker restarted, all the containers died and the IP allocations were released by the daemon. Fix this by ensuring that IPAM allocations (which are tied to the life of the pod, *not* the life of the openshift-node process) persist even if the openshift-node process restarts. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1427789
|
Tested with containerized node in docker-in-docker; /var/lib/cni is preserved across 'docker restart origin/node' invocations. |
|
@sdodson I have a release-1.5 branch ready for PR too, do you want separate PRs or is one good enough? |
|
[merge] |
|
@dcbw yes, please open one for release-1.5 |
|
Evaluated for origin merge up to b0711a5 |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to b0711a5 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/823/) (Base Commit: 87597f4) |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/830/) (Base Commit: 3e26eed) (Image: devenv-rhel7_6036) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the move to a CNI plugin, docker no longer handles IPAM, but CNI does through
openshift-sdn's usage of the 'host-local' CNI IPAM plugin. That plugin stores
IPAM allocations under /var/lib/cni/.
If the node container gets restarted, without presreving /var/lib/cni, the IPs
currently allocated to running pods get lost and on restart, openshift-sdn
may allocate those IPs to new pods causing duplicate allocations.
This never happened with docker because it has its own persistent IPAM store that
does not get removed when docker restarts. Also because (historically) when docker
restarted, all the containers died and the IP allocations were released by the
daemon.
Fix this by ensuring that IPAM allocations (which are tied to the life of the pod,
not the life of the openshift-node process) persist even if the openshift-node
process restarts.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1427789
@sdodson @openshift/networking @eparis