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

[ec2tagger] Set refresh interval for describe volumes to 5m when configured #1578

Merged
merged 13 commits into from
Mar 5, 2025

Conversation

lisguo
Copy link
Contributor

@lisguo lisguo commented Mar 3, 2025

Description of the issue

Currently refresh interval seconds is the config used for determining the interval for both tags and volumes. This PR will create two separate intervals: RefreshTagsInterval, and RefreshVolumesInterval. By default we will set the refresh interval for volumes to 5 minutes if VolumeId is configured as an append dimension. Tags will remain the same at 0.

Description of changes

  • Remove RefreshIntervalSeconds config in ec2tagger
  • Create RefreshTagsInterval and RefreshVolumesInterval in ec2tagger
  • Set the default RefreshVolumesInterval to 5 minutes in the ec2 tagger translator if VolumeId is configured in append dimensions
  • Split refreshLoopToUpdateTagsAndVolumes() function into separate funcs refreshLoopToUpdateTags() and refreshLoopToUpdateVolumes() according to the intervals
  • Update all test yaml files and unit tests

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

Unit tests

Requirements

Before commit the code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

@lisguo lisguo requested a review from a team as a code owner March 3, 2025 20:26
@lisguo lisguo requested review from jefchien and musa-asad March 4, 2025 15:20
TravisStark
TravisStark previously approved these changes Mar 4, 2025
Paramadon
Paramadon previously approved these changes Mar 4, 2025
Copy link
Contributor

@Paramadon Paramadon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lisguo
Copy link
Contributor Author

lisguo commented Mar 4, 2025

PR Build for windows is failing -- need to update windows translation unit tests

musa-asad
musa-asad previously approved these changes Mar 4, 2025
@lisguo lisguo dismissed stale reviews from musa-asad, Paramadon, and TravisStark via b106ac1 March 4, 2025 16:06
@lisguo lisguo changed the title [ec2tagger] Set refresh interval for describe tags to 5m [ec2tagger] Set refresh interval for describe volumes to 5m when configured Mar 4, 2025
@jefchien
Copy link
Contributor

jefchien commented Mar 4, 2025

Need to regenerate the sample config YAMLs.

@lisguo lisguo merged commit f51ad4a into main Mar 5, 2025
7 checks passed
@lisguo lisguo deleted the ec2tagger-volumes branch March 5, 2025 15:42
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

Successfully merging this pull request may close these issues.

5 participants