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

Wrong AMI deletion when using multiple values in --excluded-mapping-values option #95

Open
adriananeci opened this issue Nov 6, 2018 · 1 comment

Comments

@adriananeci
Copy link

When I try to pass multiple values for --excluded-mapping-values parameter I get duplicate AMIs marked for deletion on one hand and also multiple AMIs which should be excluded are wrongly added in AMIs to clean on the other hand.
amicleaner --mapping-key tags --mapping-values Scope --excluded-mapping-values dev_ready test_ready --full-report --keep-previous 0

Assuming that I have 14 AMIs with 'testing' Scope tag and 14 AMis with 'dev_ready', when I try to cleanup all 'testing' AMIs I get 28(all 14 AMIs ID are duplicated) AMIs to be removed in testing group but also few AMIs in dev_ready group which is abnormal since I excluded 'dev_ready' Scope tags.

@jhaage-jama
Copy link

I ran into this same issue. It looks like the logic here is to see if the excluded_mapping_value is not in the mapping_value (i.e. not an excluded value) then add it to the candidates. The problem with the current logic is that each AMI needs to run through each excluded mapping value. For one or more of the excluded mapping values you pass in it won't match and then will add that AMI to the candidates list. It also has the unfortunate side effect of duplicating any AMIs that weren't going to be excluded in the first place.

I worked around this by changing the strategy and pre-populating the candidates list (same as it does when there are no --excluded-mapping-values provided) but then removing any AMIs from the list if they match any of the excluded values.

This seems to fix the two issues I was seeing:

  1. Excluded AMIs still show up in the list of candidates to delete
  2. Candidates to delete are duplicated

The bit of logic that handles this is in core.py, within the map_candidates method. Here's the section of this method I modified:

if mapping_strategy.get("excluded"): mapping_list = candidates_map.get(mapping_value) or [] mapping_list.append(ami) candidates_map[mapping_value] = mapping_list for excluded_mapping_value in mapping_strategy.get("excluded"): if excluded_mapping_value in mapping_value: print("Excluding AMI from candidates list based on exclusion: {}".format(mapping_value)) mapping_list = candidates_map.get(mapping_value) or [] mapping_list.remove(ami) candidates_map[mapping_value] = mapping_list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants