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

Allow EC2 "Name" tag to be supplied in the provisioner spec #964

Merged
merged 7 commits into from
Dec 13, 2021

Conversation

mslavin-tibco
Copy link
Contributor

1. Issue, if available:
#944

2. Description of changes:
These changes allow the EC2 "Name" tag to be set in the provisioner spec as:

spec:
  provider: 
    tags:
      Name: <custom-name>

If the Name tag is not supplied in the spec, the logic falls back to the previous default value of Name: karpenter.sh/<cluster-name>

3. Does this change impact docs?

  • [ X] Yes, PR includes docs updates

4. Testing
All local tests passed

$ make test
ginkgo -r
[1639147378] Validation - 18/18 specs •••••••••••••••••• SUCCESS! 1.195387ms PASS
[1639147378] CloudProvider/AWS - 23/23 specs ••••••••••••••••••••••• SUCCESS! 30.781316015s PASS
[1639147378] Node - 18/18 specs •••••••••••••••••• SUCCESS! 10.892751879s PASS
[1639147378] Controllers/Provisioning - 11/11 specs ••••••••••• SUCCESS! 21.831135136s PASS
[1639147378] Controllers/Scheduling - 42/42 specs •••••••••••••••••••••••••••••••••••••••••S SUCCESS! 45.285634007s PASS
[1639147378] Termination - 6/6 specs •••••• SUCCESS! 10.720924721s PASS
[1639147378] Functional Suite - 10/10 specs •••••••••• SUCCESS! 497.654µs PASS

Ginkgo ran 7 suites in 2m19.232043138s
Test Suite Passed
$ make battletest
# Ensure all files have cyclo-complexity =< 10
gocyclo -over 11 ./pkg
# Run randomized, parallelized, racing, code coveraged, tests
ginkgo -r \
		-cover -coverprofile=coverage.out -outputdir=. -coverpkg=./pkg/... \
		--randomizeAllSpecs --randomizeSuites -race
[1639147539] Controllers/Scheduling - 42/42 specs •••••••••••••••••••••••••••S•••••••••••••• SUCCESS! 51.444836812s PASS
coverage: 65.4% of statements in ./pkg/...
[1639147539] Node - 18/18 specs •••••••••••••••••• SUCCESS! 12.241039979s PASS
coverage: 19.9% of statements in ./pkg/...
[1639147539] Validation - 18/18 specs •••••••••••••••••• SUCCESS! 5.150438ms PASS
coverage: 18.1% of statements in ./pkg/...
[1639147539] Termination - 6/6 specs •••••• SUCCESS! 13.146216705s PASS
coverage: 16.4% of statements in ./pkg/...
[1639147539] Controllers/Provisioning - 11/11 specs ••••••••••• SUCCESS! 24.261611134s PASS
coverage: 61.1% of statements in ./pkg/...
[1639147539] CloudProvider/AWS - 23/23 specs ••••••••••••••••••••••• SUCCESS! 35.156795382s PASS
coverage: 65.7% of statements in ./pkg/...
[1639147539] Functional Suite - 10/10 specs •••••••••• SUCCESS! 1.776086ms PASS
coverage: 66.0% of statements in ./pkg/...
path is /Users/mslavin/dev/cic2/go/src/github.com/aws/karpenter
All profiles combined

Ginkgo ran 7 suites in 2m43.801290943s
Test Suite Passed

In addition to the above local tests, the updated images were built locally and pushed to my development ECR. The updated chart was then deployed and the code changes were tested in AWS EKS.

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

@netlify
Copy link

netlify bot commented Dec 10, 2021

✔️ Deploy Preview for karpenter-docs-prod ready!

🔨 Explore the source changes: 14b509f

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/61b7b87d62c76e0008563faa

😎 Browse the preview: https://deploy-preview-964--karpenter-docs-prod.netlify.app

"Name": fmt.Sprintf("karpenter.sh/%s", clusterName),
fmt.Sprintf(ClusterTagKeyFormat, clusterName): "owned",
fmt.Sprintf(KarpenterTagKeyFormat, clusterName): "owned",
func MergeTags(ctx context.Context, customTags map[string]string) []*ec2.Tag {
Copy link
Contributor

@ellistarn ellistarn Dec 13, 2021

Choose a reason for hiding this comment

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

Optional: put this on the aws object.

func (a *AWS) GetTags()

ellistarn
ellistarn previously approved these changes Dec 13, 2021
Copy link
Contributor

@ellistarn ellistarn left a comment

Choose a reason for hiding this comment

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

Nice!

@ellistarn ellistarn enabled auto-merge (squash) December 13, 2021 21:02
auto-merge was automatically disabled December 13, 2021 21:17

Head branch was pushed to by a user without write access

@ellistarn
Copy link
Contributor

Thank you! Will cut a release soon.

@ellistarn ellistarn merged commit acbf53f into aws:main Dec 13, 2021
jkroepke added a commit to jkroepke/karpenter that referenced this pull request Apr 10, 2022
README: add link to Netlify dashboard (aws#964)
bwagner5 added a commit that referenced this pull request Apr 25, 2022
* chart: Added extraVolumes and extraVolumeMounts
README: add link to Netlify dashboard (#964)

* Update charts/karpenter/templates/deployment.yaml

Co-authored-by: Brandon Wagner <[email protected]>

* Update charts/karpenter/templates/deployment.yaml

Co-authored-by: Brandon Wagner <[email protected]>
Co-authored-by: Brandon Wagner <[email protected]>
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.

2 participants