This repository has been archived by the owner on Jan 30, 2020. It is now read-only.
fleetctl: Remove unit content from etcd #1291
Closed
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
Fleetctl destroy unit does not remove the unit file content from etcd registry(/_coreos.com/fleet/unit/XXXX). It will cause so many junk to leave in the etcd. The modification removes it from etcd when destroying unit. Fixes coreos#1290
| if u != nil { //delete unit | ||
| path := u.Unit.Hash().String() | ||
| key = r.prefixed(unitPrefix, path) | ||
| _, err = r.kAPI.Delete(r.ctx(), key, opts) |
There was a problem hiding this comment.
This isn't safe as there may be two units that hash to the same key in etcd.
|
Yes, it can be. Then we can't store the unit files using hash value, right? Maybe we can store it in the job folder. When we delete the job folder, delete the unit file at the same time. How do you think? |
|
I'd prefer to create a garbage collector that cleans up the etcd keyspace periodically. We can safely check for job models that refer to the unit hash before deleting it. |
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Mar 18, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Mar 21, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Mar 24, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Mar 24, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Apr 4, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Apr 19, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Apr 25, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Apr 29, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to dongsupark/fleet
that referenced
this pull request
May 26, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
|
Closed in favor of #1510 |
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Jul 21, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Aug 12, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
dongsupark
pushed a commit
to endocode/fleet
that referenced
this pull request
Aug 31, 2016
So far each command "fleetctl destroy unit" has removed job entries from the etcd registry, under /_coreos.com/fleet/job. But it has not removed its unit file, under /_coreos.com/fleet/unit. As a result, fleet left lots of garbages in the etcd registry, so users had to manually clean them up. So this patch gets unit contents deleted actually from etcd registry when DestroyUnit() gets called. To avoid potential hash collisions, it first fetches a list of units from registry, to check there's any duplicated entry. Only if no duplicated unit is found, fleetd actually deletes the unit from registry. Fixes: coreos#1456 Fixes: coreos#1290 Reference: coreos#1291
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fleetctl destroy unit does not remove the unit file content from
etcd registry(/_coreos.com/fleet/unit/XXXX). It will cause so
many junk to leave in the etcd. The modification removes it from
etcd when destroying unit.
Fixes #1290