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 service queries treating 404s as OK #2256

Merged
merged 1 commit into from Jun 23, 2023

Conversation

chrisdoherty4
Copy link
Member

@chrisdoherty4 chrisdoherty4 commented Jun 23, 2023

Fixes aws/eks-anywhere#5564

When the Hegel metadata service doesn't have a Hardware record it returns 404. Our writefile Tinkerbell action treats the 404 as OK resulting in invalid metadata being written to disk and ultimately consumed by kubelet on launch, causing it to fail.

This change treats non-200 responses as errors ensuring we move to query any additional metadata endpoints.

@eks-distro-bot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@eks-distro-bot eks-distro-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 23, 2023
@chrisdoherty4 chrisdoherty4 marked this pull request as ready for review June 23, 2023 16:47
When the Hegel metadata service doesn't have a Hardware record it
returns 404. Our writefile Tinkerbell action treats the 404 as OK
resulting in invalid metadata being written to disk and ultimately
consumed by kubelet on launch, causing it to fail.

This change treats non-200 responses as errors ensuring we move to
query any additional metadata endpoints.
@eks-distro-bot eks-distro-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 23, 2023
@chrisdoherty4
Copy link
Member Author

/approve

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrisdoherty4

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot merged commit 449df2f into aws:main Jun 23, 2023
2 checks passed

+ // Ensure non-200 responses are considered an error and move to the next metadata server.
+ if resp.StatusCode != http.StatusOK {
+ continue
Copy link
Member

Choose a reason for hiding this comment

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

How is it that we can use continue here? Is this within some kind of loop? I'm not too familiar with the syntax in patches.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, exactly. its in a for loop. And i only know that because im familiar with the code base. Unfortunately, with the way we do patches you're not able to see this context in the PR. And to see the proper context, you'd have to apply all our patches first. I.e. clone this repo and run then make clone-repo, make checkout-repo, and make patch-repo. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
4 participants