plume prune enhancement #343
Merged
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
|
Here's the log output for a dry-run on the edge channel: |
jepio
force-pushed
the
jepio/plume-prune-enhancement
branch
from
6d97932
to
a02cda8
Compare
Needed to access "lastLaunchedTime" attribute on AMIs.
jepio
force-pushed
the
jepio/plume-prune-enhancement
branch
from
a02cda8
to
51a9e08
Compare
krnowak
reviewed
Jul 5, 2022
jepio
force-pushed
the
jepio/plume-prune-enhancement
branch
2 times, most recently
from
23e480a
to
f090429
Compare
krnowak
approved these changes
Jul 7, 2022
|
@tormath1 would love it if you would take a look as well |
tormath1
reviewed
Jul 8, 2022
| @@ -56,6 +67,18 @@ func runPrune(cmd *cobra.Command, args []string) { | |||
| if len(args) > 0 { | |||
| plog.Fatal("No args accepted") | |||
| } | |||
| if daysLastLaunched < 0 { | |||
There was a problem hiding this comment.
According to the helper:
Only used when --check-last-launched is set.
Even if the default value is 0, we could add an extra condition like you're doing later:
Suggested change
| if daysLastLaunched < 0 { | |
| if checkLastLaunched && daysLastLaunched < 0 { |
EDIT: we could even group the two statements:
Suggested change
| if daysLastLaunched < 0 { | |
| if checkLastLaunched && (daysLastLaunched < 0 || daysLastLaunched == 0) { | |
| daysLastLaunched = days |
There was a problem hiding this comment.
I think I would rather keep the if-blocks separate so that we can report a clear error messages to users. But you made me realize I need at least one more check: if !checkLastLaunched && daysLastLaunched > 0 so that we tell users that the option will be ignored.
Images are first marked "soft-deleted" by removing launchpermissions from them. If a "soft-deleted" image is found again, it is permanently deleted after a grace period expires. Soft-deleting has the advantage that we do not invalidate ami ids.
jepio
force-pushed
the
jepio/plume-prune-enhancement
branch
from
f090429
to
d698ec3
Compare
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.
plume prune enhancements
How to use
Testing done
Tested with dry-run only, would run on "edge" channel first.
changelog/directory (user-facing change, bug fix, security fix, update)