8000 Store 'checkAlive' flag in IPAM, so we don't delete non-container IDs by bboreham · Pull Request #2165 · weaveworks/weave · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jun 20, 2024. It is now read-only.

Store 'checkAlive' flag in IPAM, so we don't delete non-container IDs #2165

Merged
merged 4 commits into from
Apr 18, 2016
Merged

Store 'checkAlive' flag in IPAM, so we don't delete non-container IDs #2165

merged 4 commits into from
Apr 18, 2016

Conversation

bboreham
Copy link
Contributor

Fixes #2164

Note the owned map changes type so breaks compatibility with earlier persistence files

@bboreham bboreham added this to the 1.5.0 milestone Apr 14, 2016
@awh awh self-assigned this Apr 14, 2016
@awh
Copy link
Contributor
awh commented Apr 15, 2016 •
edited
Loading

Two thoughts

  1. This requires lots of change for little effect - I wonder if we shouldn't revisit your idea of detecting whether the container name is valid (weave:expose and weave:extern were carefully chosen to be the same length as a short container ID but not be valid container names or IDs; furthermore the _ addresses which are stored using the CIDR as the container name aren't valid either). See https://github.com/docker/docker/blob/master/utils/names.go
  2. If we do go this route, should checkAlive really be something like isContainerAddress, at least in the persisted data?

@awh awh removed their assignment Apr 15, 2016
@awh
Copy link
Contributor
awh commented Apr 15, 2016

So it would seem that checkAlive was added in the first place due to objections about inferring from magic values, so I guess we shouldn't do that here. Should consider renaming field as per (2) above. Needs rebase.

@bboreham bboreham force-pushed the issues/2164-less-deleting branch from 23326ee to 1764661 Compare April 15, 2016 17:00
@bboreham
Copy link
Contributor Author

Renamed and rebased

@bboreham bboreham removed their assignment Apr 15, 2016
@bboreham bboreham force-pushed the issues/2164-less-deleting branch from 1764661 to 5e4ab7f Compare April 18, 2016 09:18
@awh awh self-assigned this Apr 18, 2016
@awh awh merged commit 6ad9dac into master Apr 18, 2016
@awh awh deleted the issues/2164-less-deleting branch April 18, 2016 10:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@awh awh

Labels
None yet
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@bboreham @awh
0