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

fix: metadata TTL #164

Merged
merged 1 commit into from
Jan 12, 2024
Merged

fix: metadata TTL #164

merged 1 commit into from
Jan 12, 2024

Conversation

shanduur
Copy link
Member

@shanduur shanduur commented Jan 10, 2024

General:

  • Have you removed all sensitive information, including but not limited to access keys and passwords?
  • Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. Does your submission pass tests?
  2. Have you added tests?
  3. Are you addressing a single feature in this PR?
  4. Are your commits atomic, addressing one change per commit?
  5. Are you following the conventions of the language?
  6. Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. Have you explained your rationale for why this feature is needed?
  8. Have you linked your PR to an open issue

BUGFIX: This code change involves modifying the logic for handling metadata updates for Linode instances in a Kubernetes cluster. Previous behaviour spammed, did redundant work on Create and Update, and every time it was triggered, it pulled the node from the API. This prevents hitting the API too often.

  1. instances.go:

    • The timeout variable is changed to be initialised with a default value of 15.
    • If the environment variable LINODE_INSTANCE_CACHE_TTL is set, the timeout is updated accordingly, but now it checks if the parsed value is greater than 0 before updating the timeout. This ensures that only positive values are considered valid for the timeout.
  2. node_controller.go:

    • The nodeController structure is extended to include a sync.RWMutex for safe concurrent access to shared data.
    • Two new fields, metadataLastUpdate and ttl, are added to keep track of the last metadata update time and the time-to-live for metadata, respectively.
    • The timeout variable is introduced, which defaults to 300 seconds (5 minutes), and it can be overridden by the LINODE_METADATA_TTL environment variable.
    • Two new methods, LastMetadataUpdate and SetLastMetadataUpdate, are added to get and set the last metadata update time for a given node name in a thread safe manner.

    Changes in handleNodeAdded:

    • Before attempting to fetch Linode information, it checks the last update time and the TTL to decide whether a fresh metadata update is required.
    • The SetLastMetadataUpdate method is called after a successful update to store the current time as the last update time for that node.

    Changes in Run:

    • The UpdateFunc in the informer is removed. This function was handling updated nodes' metadata, but it's redundant.

In summary, the changes provide better control over the caching mechanism for Linode instance metadata. The introduction of a TTL for metadata and the ability to configure it through environment variables allow for more flexibility and optimisation in handling updates. Additionally, the use of a mutex ensures thread-safe access to shared data in a concurrent environment.

@shanduur shanduur added the bugfix for any bug fixes in the changelog. label Jan 10, 2024
@shanduur shanduur force-pushed the fix/metadata-ttl branch 2 times, most recently from baeb1ac to a27ec3a Compare January 10, 2024 10:56
@srust srust merged commit 74cd6d2 into linode:main Jan 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix for any bug fixes in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants