diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 66f94fbb6a..f841ea719f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,9 +1,12 @@ ### Description -Please explain the changes you made here. + + ### Checklist + - [ ] Code compiles correctly (i.e `make build`) - [ ] Added tests that cover your change (if possible) -- [ ] All tests passing (i.e. `make test`) -- [ ] Added/modified documentation as required (such as the README) +- [ ] All unit tests passing (i.e. `make test`) +- [ ] All integration tests passing (i.e. `make integration-test`) +- [ ] Added/modified documentation as required (such as the `README.md`, and `examples` directory) - [ ] Added yourself to the `humans.txt` file diff --git a/.gitignore b/.gitignore index c01a5f4f49..2bf11af307 100644 --- a/.gitignore +++ b/.gitignore @@ -4,8 +4,9 @@ # Release config file backups .goreleaser.brew.combined.yml -# Locally built binary +# Locally built binaries /eksctl +/eksctl-integration-test # Bad deps vendor/k8s.io/kops/docs diff --git a/.gometalinter.json b/.gometalinter.json index 6657f7bbaa..f64b729ab6 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -3,13 +3,11 @@ "vet", "golint", "errcheck", - "deadcode" + "deadcode", + "misspell" ], "Exclude": [ - "^vendor\/", - "^build\/", "^pkg\/nodebootstrap\/assets.go", - "\/usr\/local\/go", "^pkg\/testutils\/ginkgo.go" ], "Deadline": "5m" diff --git a/Dockerfile b/Dockerfile index 46e0b162aa..2d108a7d9e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,9 +36,12 @@ RUN mkdir -p "${JUNIT_REPORT_DIR}" WORKDIR $EKSCTL RUN make $TEST_TARGET -RUN make lint RUN make build \ && cp ./eksctl /out/usr/local/bin/eksctl +RUN make build-integration-test \ + && mkdir -p /out/usr/local/share/eksctl \ + && cp integration/*.yaml /out/usr/local/share/eksctl \ + && cp ./eksctl-integration-test /out/usr/local/bin/eksctl-integration-test RUN go build ./vendor/github.com/kubernetes-sigs/aws-iam-authenticator/cmd/aws-iam-authenticator \ && cp ./aws-iam-authenticator /out/usr/local/bin/aws-iam-authenticator diff --git a/Makefile b/Makefile index 4ccf069e6a..ab531ff10a 100644 --- a/Makefile +++ b/Makefile @@ -8,14 +8,6 @@ EKSCTL_IMAGE ?= weaveworks/eksctl:latest .DEFAULT_GOAL := help -ifneq ($(TEST_V),) -TEST_ARGS ?= -v -ginkgo.v -endif - -ifneq ($(DELETE_TEST_CLUSTER),) -INTEGRATION_TEST_ARGS ?= -eksctl.delete=false -endif - ##@ Dependencies .PHONY: install-build-deps @@ -30,34 +22,58 @@ build: ## Build eksctl ##@ Testing & CI +ifneq ($(TEST_V),) +UNIT_TEST_ARGS ?= -v -ginkgo.v +INTEGRATION_TEST_ARGS ?= -test.v -ginkgo.v +endif + +LINTER ?= gometalinter ./pkg/... ./cmd/... ./integration/... +.PHONY: lint +lint: ## Run linter over the codebase + @$(GOPATH)/bin/$(LINTER) + .PHONY: test test: generate ## Run unit test (and re-generate code under test) + @$(MAKE) lint @git diff --exit-code pkg/nodebootstrap/assets.go > /dev/null || (git --no-pager diff pkg/nodebootstrap/assets.go; exit 1) @git diff --exit-code ./pkg/eks/mocks > /dev/null || (git --no-pager diff ./pkg/eks/mocks; exit 1) @$(MAKE) unit-test @test -z $(COVERALLS_TOKEN) || $(GOPATH)/bin/goveralls -coverprofile=coverage.out -service=circle-ci - @go test -tags integration ./integration/... -c && rm -f integration.test + @$(MAKE) build-integration-test .PHONY: unit-test unit-test: ## Run unit test only - @CGO_ENABLED=0 go test -covermode=count -coverprofile=coverage.out ./pkg/... ./cmd/... $(TEST_ARGS) + @CGO_ENABLED=0 go test -covermode=count -coverprofile=coverage.out ./pkg/... ./cmd/... $(UNIT_TEST_ARGS) -LINTER ?= gometalinter ./... -.PHONY: lint -lint: ## Run linter over the codebase - @$(GOPATH)/bin/$(LINTER) +.PHONY: build-integration-test +build-integration-test: ## Build integration test binary + @go test -tags integration ./integration/... -c -o ./eksctl-integration-test -.PHONY: ci -ci: test lint ## Target for CI system to invoke to run tests and linting +.PHONY: integration-test +integration-test: build build-integration-test ## Run the integration tests (with cluster creation and cleanup) + @./eksctl-integration-test -test.timeout 60m \ + $(INTEGRATION_TEST_ARGS) + +.PHONY: integration-test-container +integration-test-container: eksctl-image ## Run the integration tests inside a Docker container + @docker run \ + --env=AWS_PROFILE \ + --volume=$(HOME)/.aws:/root/.aws \ + --workdir=/usr/local/share/eksctl \ + $(EKSCTL_IMAGE) \ + eksctl-integration-test \ + -eksctl.path=/usr/local/bin/eksctl \ + -eksctl.kubeconfig=/tmp/kubeconfig \ + $(INTEGRATION_TEST_ARGS) TEST_CLUSTER ?= integration-test-dev .PHONY: integration-test-dev -integration-test-dev: build ## Run the integration tests without cluster teardown. For use when developing integration tests. +integration-test-dev: build build-integration-test ## Run the integration tests without cluster teardown. For use when developing integration tests. @./eksctl utils write-kubeconfig \ --auto-kubeconfig \ --name=$(TEST_CLUSTER) - @go test -tags integration -timeout 21m ./integration/... \ - $(TEST_ARGS) \ + @./eksctl-integration-test -test.timeout 21m \ + $(INTEGRATION_TEST_ARGS) \ -args \ -eksctl.cluster=$(TEST_CLUSTER) \ -eksctl.create=false \ @@ -70,10 +86,6 @@ create-integration-test-dev-cluster: build ## Create a test cluster for use when delete-integration-test-dev-cluster: build ## Delete the test cluster for use when developing integration tests @./eksctl delete cluster --name=integration-test-dev --auto-kubeconfig -.PHONY: integration-test -integration-test: build ## Run the integration tests (with cluster creation and cleanup) - @go test -tags integration -timeout 60m ./integration/... $(TEST_ARGS) $(INTEGRATION_TEST_ARGS) - ##@ Code Generation .PHONY: generate diff --git a/pkg/apis/eksctl.io/v1alpha4/validation.go b/pkg/apis/eksctl.io/v1alpha4/validation.go index 60a0766d36..3f3c9a195f 100644 --- a/pkg/apis/eksctl.io/v1alpha4/validation.go +++ b/pkg/apis/eksctl.io/v1alpha4/validation.go @@ -30,8 +30,8 @@ func validateNodeGroupIAM(i int, ng *NodeGroup, value, fieldName, path string) e } // ValidateNodeGroupLabels uses proper Kubernetes label validation, -// it's desinged to make sure users don't pass weird labels to the -// nodes, which would prevent kubelets to startup propertly +// it's designed to make sure users don't pass weird labels to the +// nodes, which would prevent kubelets to startup properly func ValidateNodeGroupLabels(ng *NodeGroup) error { // compact version based on: // - https://github.com/kubernetes/kubernetes/blob/v1.13.2/cmd/kubelet/app/options/options.go#L257-L267 diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go index b7df209fde..03500defd0 100644 --- a/pkg/cfn/builder/api_test.go +++ b/pkg/cfn/builder/api_test.go @@ -486,7 +486,7 @@ var _ = Describe("CloudFormation template builder API", func() { })) }) - It("should have auto-dicovery tags", func() { + It("should have auto-discovery tags", func() { expectedTags := []Tag{ { Key: "Name", diff --git a/pkg/cfn/builder/iam.go b/pkg/cfn/builder/iam.go index 6d9113c219..a06c40f1a8 100644 --- a/pkg/cfn/builder/iam.go +++ b/pkg/cfn/builder/iam.go @@ -152,7 +152,7 @@ func (n *NodeGroupResourceSet) addResourcesForIAM() { // if neither role nor profile are given - create both if n.spec.IAM.InstanceRoleName == "" { - // setting role name requires addtional capabilities + // setting role name requires additional capabilities n.rs.withNamedIAM = true } diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index e7ce244b68..f8843f6ecd 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -211,7 +211,17 @@ func (c *StackCollection) DeleteStack(name string) (*Stack, error) { i := &Stack{StackName: &name} s, err := c.describeStack(i) if err != nil { - return nil, errors.Wrapf(err, "not able to get stack %q for deletion", name) + err = errors.Wrapf(err, "not able to get stack %q for deletion", name) + stacks, newErr := c.ListStacks(fmt.Sprintf("^%s$", name), cloudformation.StackStatusDeleteComplete) + if newErr != nil { + logger.Critical("not able double-check if stack was already deleted: %s", newErr.Error()) + } + if count := len(stacks); count > 0 { + logger.Debug("%d deleted stacks found {%v}", count, stacks) + logger.Info("stack %q was already deleted", name) + return nil, nil + } + return nil, err } i.StackId = s.StackId for _, tag := range s.Tags { diff --git a/pkg/cfn/outputs/api.go b/pkg/cfn/outputs/api.go index 841c533387..6b929f7774 100644 --- a/pkg/cfn/outputs/api.go +++ b/pkg/cfn/outputs/api.go @@ -41,14 +41,14 @@ type ( // Collector is a callback function that takes an output value // and may return an error Collector func(string) error - // Collectors are a map of ouput names to Collector callbacks + // Collectors are a map of output names to Collector callbacks collectors = map[string]Collector // CollectorSet is a wrapper to define methods for collectors CollectorSet struct{ set collectors } ) // NewCollectorSet creates a new CollectorSet based on a map of -// ouput names to Collector callbacks +// output names to Collector callbacks func NewCollectorSet(set map[string]Collector) *CollectorSet { if set == nil { return &CollectorSet{make(collectors)} @@ -67,7 +67,7 @@ func (c *CollectorSet) doCollect(must bool, stack cfn.Stack) error { } if value == nil { if must { - err := fmt.Errorf("no ouput %q", key) + err := fmt.Errorf("no output %q", key) if stack.StackName != nil { return fmt.Errorf("%s in stack %q", err.Error(), *stack.StackName) } diff --git a/pkg/cfn/outputs/api_test.go b/pkg/cfn/outputs/api_test.go index 5e89d2fd5c..0f88f05e50 100644 --- a/pkg/cfn/outputs/api_test.go +++ b/pkg/cfn/outputs/api_test.go @@ -42,7 +42,7 @@ var _ = Describe("CloudFormation stack outputs API", func() { } err := Collect(stack, requiredCollectors, nil) Expect(err).Should(HaveOccurred()) - Expect(err.Error()).To(Equal("no ouput \"" + ClusterVPC + "\"")) + Expect(err.Error()).To(Equal("no output \"" + ClusterVPC + "\"")) } { @@ -65,7 +65,7 @@ var _ = Describe("CloudFormation stack outputs API", func() { err := Collect(stack, requiredCollectors, nil) Expect(err).Should(HaveOccurred()) - Expect(err.Error()).To(Equal("no ouput \"" + ClusterSecurityGroup + "\" in stack \"foo\"")) + Expect(err.Error()).To(Equal("no output \"" + ClusterSecurityGroup + "\" in stack \"foo\"")) } appendOutput(&stack, ClusterSecurityGroup, "sg-123") @@ -109,7 +109,7 @@ var _ = Describe("CloudFormation stack outputs API", func() { } err := Collect(stack, requiredCollectors, nil) Expect(err).Should(HaveOccurred()) - Expect(err.Error()).To(Equal("no ouput \"" + ClusterVPC + "\"")) + Expect(err.Error()).To(Equal("no output \"" + ClusterVPC + "\"")) } { diff --git a/pkg/ctl/cmdutils/nodegroup.go b/pkg/ctl/cmdutils/nodegroup.go index 8b9ef55281..f4550153f9 100644 --- a/pkg/ctl/cmdutils/nodegroup.go +++ b/pkg/ctl/cmdutils/nodegroup.go @@ -45,7 +45,7 @@ func AddCommonCreateNodeGroupFlags(cmd *cobra.Command, fs *pflag.FlagSet, p *api fs.StringVar(&ng.SSHPublicKeyPath, "ssh-public-key", defaultSSHPublicKey, "SSH public key to use for nodes (import from local path, or use existing EC2 key pair)") fs.StringVar(&ng.AMI, "node-ami", ami.ResolverStatic, "Advanced use cases only. If 'static' is supplied (default) then eksctl will use static AMIs; if 'auto' is supplied then eksctl will automatically set the AMI based on version/region/instance type; if any other value is supplied it will override the AMI to use for the nodes. Use with extreme care.") - fs.StringVar(&ng.AMIFamily, "node-ami-family", ami.ImageFamilyAmazonLinux2, "Advanced use cases only. If 'AmazonLinux2' is supplied (default), then eksctl will use the offical AWS EKS AMIs (Amazon Linux 2); if 'Ubuntu1804' is supplied, then eksctl will use the offical Canonical EKS AMIs (Ubuntu 18.04).") + fs.StringVar(&ng.AMIFamily, "node-ami-family", ami.ImageFamilyAmazonLinux2, "Advanced use cases only. If 'AmazonLinux2' is supplied (default), then eksctl will use the official AWS EKS AMIs (Amazon Linux 2); if 'Ubuntu1804' is supplied, then eksctl will use the official Canonical EKS AMIs (Ubuntu 18.04).") fs.BoolVarP(&ng.PrivateNetworking, "node-private-networking", "P", false, "whether to make nodegroup networking private") @@ -60,9 +60,9 @@ func AddCommonCreateNodeGroupIAMAddonsFlags(fs *pflag.FlagSet, ng *api.NodeGroup fs.StringSliceVar(&ng.IAM.AttachPolicyARNs, "temp-node-role-policies", []string{}, "Advanced use cases only. "+ "All the IAM policies to be associated to the node's instance role. "+ "Beware that you MUST include the policies for EKS and CNI related AWS API Access, like `arn:aws:iam::aws:policy/AmazonEKSWorkerNodePolicy` and `arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy` that are used by default when this flag is omitted.") - fs.MarkHidden("temp-node-role-policies") + _ = fs.MarkHidden("temp-node-role-policies") fs.StringVar(&ng.IAM.InstanceRoleName, "temp-node-role-name", "", "Advanced use cases only. Specify the exact name of the node's instance role for easier integration with K8S-IAM integrations like kube2iam. See https://github.com/weaveworks/eksctl/issues/398 for more information.") - fs.MarkHidden("temp-node-role-name") + _ = fs.MarkHidden("temp-node-role-name") ng.IAM.WithAddonPolicies.AutoScaler = new(bool) ng.IAM.WithAddonPolicies.ExternalDNS = new(bool) diff --git a/pkg/ctl/completion/completion.go b/pkg/ctl/completion/completion.go index 6f91851fbe..139ea1c718 100644 --- a/pkg/ctl/completion/completion.go +++ b/pkg/ctl/completion/completion.go @@ -21,8 +21,8 @@ To configure your bash shell to load completions for each session add to your ba # ~/.bashrc or ~/.profile . <(eksctl completion bash) `, - Run: func(cmd *cobra.Command, args []string) { - rootCmd.GenBashCompletion(os.Stdout) + RunE: func(cmd *cobra.Command, args []string) error { + return rootCmd.GenBashCompletion(os.Stdout) }, } var zshCompletionCmd = &cobra.Command{ @@ -38,8 +38,8 @@ and put the following in ~/.zshrc: fpath=($fpath ~/.zsh/completion) `, - Run: func(cmd *cobra.Command, args []string) { - rootCmd.GenZshCompletion(os.Stdout) + RunE: func(cmd *cobra.Command, args []string) error { + return rootCmd.GenZshCompletion(os.Stdout) }, } diff --git a/pkg/eks/nodegroup.go b/pkg/eks/nodegroup.go index 4edead4452..1335181836 100644 --- a/pkg/eks/nodegroup.go +++ b/pkg/eks/nodegroup.go @@ -45,7 +45,9 @@ func (c *ClusterProvider) CreateOrUpdateNodeGroupAuthConfigMap(clientSet *client return nil } - utils.UpdateAuthConfigMap(cm, ng.IAM.InstanceRoleARN) + if err := utils.UpdateAuthConfigMap(cm, ng.IAM.InstanceRoleARN); err != nil { + return errors.Wrap(err, "creating an update for auth ConfigMap") + } if _, err := client.Update(cm); err != nil { return errors.Wrap(err, "updating auth ConfigMap") }