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: only warn for ssm query errors #4128

Merged
merged 2 commits into from
Jun 29, 2023
Merged

fix: only warn for ssm query errors #4128

merged 2 commits into from
Jun 29, 2023

Conversation

everpcpc
Copy link
Contributor

Description
In some region without specific AMI such as cn-northwest-1 , launching would fail with errors like:

2023-06-26T08:13:14.481Z	ERROR	controller.provisioner	launching machine, creating cloud provider instance, creating instance, getting launch template configs, getting launch templates, getting ssm parameter "/aws/service/bottlerocket/aws-k8s-1.26-nvidia/arm64/latest/image_id", ParameterNotFound: 	{"commit": "698f22f-dirty"}

This error should be ignored since we do not need this kind of AMIs.

How was this change tested?

Tested with AWS cn-northwest-1

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@everpcpc everpcpc requested a review from a team as a code owner June 26, 2023 08:23
@netlify
Copy link

netlify bot commented Jun 26, 2023

Deploy Preview for karpenter-docs-prod canceled.

Name Link
🔨 Latest commit 177e41d
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/649dd42e888d3f00081c700c

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

Just a couple comments; otherwise, LGTM

pkg/providers/amifamily/ami.go Outdated Show resolved Hide resolved
pkg/providers/amifamily/ami.go Outdated Show resolved Hide resolved
@jonathan-innis
Copy link
Contributor

Would you be willing to add a functional unit test for this new scenario where the SSM alias isn't available but some other alias is available within the ami_test.go file as well?

@jonathan-innis
Copy link
Contributor

Would you be willing to add a functional unit test for this new scenario where the SSM alias isn't available but some other alias is available within the ami_test.go file as well?

Went ahead and added some functional tests around this functionality

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

/karpenter snapshot

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

/karpenter snapshot

@github-actions
Copy link
Contributor

Snapshot successfully published to oci://public.ecr.aws/karpenter/karpenter:v0-177e41dbecc27fd64c0199dd68b65dafd5674d77. Find the image tag and installation instructions at https://gallery.ecr.aws/karpenter/karpenter/

njtran
njtran previously approved these changes Jun 29, 2023
@jonathan-innis jonathan-innis enabled auto-merge (squash) June 29, 2023 22:53
@jonathan-innis jonathan-innis merged commit 16faeda into aws:main Jun 29, 2023
38 checks passed
@everpcpc everpcpc deleted the fix-ami branch July 1, 2023 01:59
@everpcpc
Copy link
Contributor Author

everpcpc commented Jul 1, 2023

Sorry for missing notification on it, thanks a lot for helping went this ahead!

johngmyers added a commit to johngmyers/karpenter that referenced this pull request May 31, 2024
Merge in DEL/karpenter-fork from upgrade-29-0 to main

* commit 'c6dd8b2cc1b0f5267a27c2b89380381e9a4d39c6': (95 commits)
  update to v0.29.0 karpenter-core (aws#4218)
  update karpenter-core (aws#4215)
  docs: Add karpenter working group call link to readme (aws#4214)
  Revert "feat: publish instanceType, capacityType, availabilityZone in a k8s event when we receive a IsUnfulfillableCapacity error while trying to create fleet " (aws#4212)
  test: Add more regions for e2etests (aws#4204)
  docs: Fix v0.28.0 version compatibility mention in the docs (aws#4203)
  chore: Update data from AWS APIs (aws#4191)
  chore(deps): bump github.com/onsi/ginkgo/v2 from 2.9.7 to 2.11.0 (aws#4184)
  chore(deps): bump github.com/aws/aws-sdk-go from 1.44.273 to 1.44.294 (aws#4185)
  chore(deps): bump github.com/onsi/gomega from 1.27.7 to 1.27.8 (aws#4183)
  chore(deps): bump golang.org/x/sync from 0.2.0 to 0.3.0 (aws#4182)
  docs: Fix guidance on ALB and `securityGroupSelector` (aws#4181)
  docs: Remove tag restriction scoping on policy (aws#4180)
  docs: Add new DaemonSet FAQ for new DaemonSets deployed after Nodes exist (aws#4161)
  chore: Remove tekton directories and references (aws#4179)
  test: e2e prometheus endpoint metrics scraping (aws#4178)
  chore: Update stalebot labels to add back `roadmap` (aws#4177)
  test: Add check for snapshot release run (aws#4175)
  fix: only warn for ssm query errors (aws#4128)
  test: Add `sts:AssumeRole` for Root in IAM (aws#4172)
  ...
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

3 participants