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

separate KMS into its own suite and reduce nodegroup creation time #4900

Merged
merged 5 commits into from
Mar 7, 2022

Conversation

aclevername
Copy link
Contributor

@aclevername aclevername commented Mar 4, 2022

Description

Closes #4847

Passed locally, got a full run queue'd https://github.com/weaveworks/eksctl-ci/actions/runs/1934052828

@aclevername aclevername added the skip-release-notes Causes PR not to show in release notes label Mar 4, 2022
Comment on lines -169 to -181
Context("Creating a cluster without --fargate", func() {
ft := &fargateTest{}

BeforeEach(func() {
setup(ft)
})

It("should allow creation of new Fargate profiles", func() {
testCreateFargateProfile(ft.clusterName, ft.kubeTest)
})

AfterEach(func() {
deleteCluster(ft.clusterName)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tested already in unowned suite

Comment on lines -152 to -167
Context("Creating a cluster with --fargate and --managed", func() {
ft := &fargateTest{}

BeforeEach(func() {
setup(ft, "--fargate", "--managed")
})

It("should support Fargate", func() {
testDefaultFargateProfile(ft.clusterName, ft.kubeTest)
testCreateFargateProfile(ft.clusterName, ft.kubeTest)
})

AfterEach(func() {
deleteCluster(ft.clusterName)
})
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't really get the point of this tbh why does testing fargate when a managed nodegroup exists need its own test? Its also tested in unowned suite

Copy link
Contributor

@Skarlso Skarlso left a comment

Choose a reason for hiding this comment

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

Noice!

@aclevername aclevername marked this pull request as ready for review March 4, 2022 15:29
Copy link
Contributor

@Himangini Himangini left a comment

Choose a reason for hiding this comment

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

👍🏻

@aclevername aclevername enabled auto-merge (squash) March 7, 2022 17:11
@aclevername aclevername merged commit 2d5e62b into eksctl-io:main Mar 7, 2022
@hspencer77 hspencer77 mentioned this pull request Jul 8, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-release-notes Causes PR not to show in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unowned test is running for over two hours
3 participants