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

Lookup linode by IP instead if label or providerID do not match #198

Merged
merged 7 commits into from
Apr 17, 2024

Conversation

eljohnson92
Copy link
Contributor

@eljohnson92 eljohnson92 commented Apr 8, 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?

This change adds a lookupByIP function if a linode cannot be found by label or ID. This means rather that as long as the node and linode share an IP address it will be able to lookup the linode. additionally, we now set the ProviderID based on this lookup if it is not already set so future lookups only need to be made against the ID.

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

@eljohnson92
Copy link
Contributor Author

one outstanding question here is do we need to be worried about collisions with linode private IPs possibly being the same with customer linodes in different DCs? perhaps we need to restrict it to just the public IP of the linode?

cloud/linode/instances.go Outdated Show resolved Hide resolved
@eljohnson92 eljohnson92 changed the title Lookup linode by IP instead of label if ProviderID is not set Lookup linode by IP instead if label or providerID do not match Apr 8, 2024
cloud/linode/instances.go Outdated Show resolved Hide resolved
cloud/linode/instances.go Outdated Show resolved Hide resolved
cloud/linode/instances.go Show resolved Hide resolved
Copy link
Contributor

@rahulait rahulait left a comment

Choose a reason for hiding this comment

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

The PR looks good to me. We might have to rebase again as we merged other PR to main.

@eljohnson92 eljohnson92 merged commit cb5d740 into main Apr 17, 2024
3 checks passed
@eljohnson92 eljohnson92 deleted the lookup_node_by_ip branch April 17, 2024 14:21
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.

None yet

4 participants