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.

Sign up for GitHub

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

Jump to bottom

Test: Fixed a few bugs with running integration tests in the integration environment #2184

Merged
merged 3 commits into from
Jul 26, 2022
Merged

Test: Fixed a few bugs with running integration tests in the integration environment #2184

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1c4f38
Test: Add explicit tolerations in test setup since snapshot releases …
ellistarn Jul 23, 2022
4eca05a
Restored clustername discovery
ellistarn Jul 25, 2022
123349b
PR Comments
ellistarn Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions test/pkg/environment/environment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
loggingtesting "knative.dev/pkg/logging/testing"
"knative.dev/pkg/ptr"
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -25,7 +26,7 @@ import (
"github.com/aws/karpenter/pkg/utils/project"
)

var clusterNameFlag = *flag.String("cluster-name", env.WithDefaultString("CLUSTER_NAME", ""), "Cluster name enables discovery of the testing environment")
var clusterNameFlag = flag.String("cluster-name", env.WithDefaultString("CLUSTER_NAME", ""), "Cluster name enables discovery of the testing environment")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this using flag? It looks like this will only ever get the env variable since you never call flag.Parse. Might as well just use:

Suggested change
var clusterNameFlag = flag.String("cluster-name", env.WithDefaultString("CLUSTER_NAME", ""), "Cluster name enables discovery of the testing environment")
var clusterName = env.WithDefaultString("CLUSTER_NAME", "")

or directly in DiscoverClusterName()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test library calls flags.Parse() for us =/. I can't call it again without creating problem. I like your solution though.


type Environment struct {
context.Context
Expand Down Expand Up @@ -63,10 +64,10 @@ func NewEnvironment(t *testing.T) (*Environment, error) {
}

func DiscoverClusterName(config *rest.Config) (string, error) {
if clusterNameFlag != "" {
return clusterNameFlag, nil
if ptr.StringValue(clusterNameFlag) != "" {
return ptr.StringValue(clusterNameFlag), nil
}
if len(config.ExecProvider.Args) > 5 {
if config.ExecProvider != nil && len(config.ExecProvider.Args) > 5 {
return config.ExecProvider.Args[5], nil
}
return "", fmt.Errorf("-cluster-name is not set and could not be discovered")
Expand Down
2 changes: 0 additions & 2 deletions test/pkg/environment/expectations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package environment

import (
"bytes"
"context"
"fmt"
"io"
"strings"
Expand Down Expand Up @@ -124,7 +123,6 @@ func (env *Environment) eventuallyExpectScaleDown() {
// expect the current node count to be what it was when the test started
g.Expect(env.Monitor.NodeCount()).To(Equal(env.Monitor.NodeCountAtReset()))
}).Should(Succeed(), fmt.Sprintf("expected scale down to %d nodes, had %d", env.Monitor.NodeCountAtReset(), env.Monitor.NodeCount()))
logging.FromContext(context.Background()).Info(env.Monitor.NodeCountAtReset())
}

func (env *Environment) ExpectCreatedNodeCount(comparator string, nodeCount int) {
Expand Down
3 changes: 1 addition & 2 deletions test/suites/common/run-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
Expand Down Expand Up @@ -30,4 +29,4 @@ spec:
workingDir: $(workspaces.ws.path)
script: |
aws eks update-kubeconfig --name $(params.cluster-name)
TEST_FILTER=$(params.test-filter) make e2etests
KUBECONFIG=/root/.kube/config TEST_FILTER=$(params.test-filter) make e2etests
4 changes: 1 addition & 3 deletions test/suites/common/setup.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,8 @@ spec:
workingDir: $(workspaces.ws.path)
script: |
aws eks update-kubeconfig --name $(params.cluster-name)
helm repo add karpenter https://charts.karpenter.sh/
helm repo update
helm upgrade --install --kubeconfig /root/.kube/config --namespace karpenter --create-namespace \
karpenter karpenter/karpenter \
karpenter oci://public.ecr.aws/karpenter/karpenter \
--version v0-$(git rev-parse HEAD) \
--set serviceAccount.annotations."eks\.amazonaws\.com/role-arn"="arn:aws:iam::$(cat $(results.account-id.path)):role/$(params.cluster-name)-karpenter" \
--set clusterName=$(params.cluster-name) \
Expand Down
2 changes: 1 addition & 1 deletion test/suites/integration/launchtemplates_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var _ = Describe("LaunchTemplates", func() {
SubnetSelector: map[string]string{"karpenter.sh/discovery": env.ClusterName},
AMIFamily: &awsv1alpha1.AMIFamilyAL2,
},
AMISelector: map[string]string{"aws-ids": customAMI},
AMISelector: map[string]string{"aws-ids": customAMI},
})
provisioner := test.Provisioner(test.ProvisionerOptions{ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name}})
pod := test.Pod()
Expand Down
5 changes: 5 additions & 0 deletions test/tools/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,8 @@ RUN aws --version
# Install eksctl
RUN curl --silent --location "https://github.com/weaveworks/eksctl/releases/download/v0.105.0/eksctl_$(uname -s)_amd64.tar.gz" | tar xz -C /tmp
RUN mv /tmp/eksctl /usr/local/bin

# Install toolchain
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like you're just caching the modules not installing the "toolchain"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I was originally going to install the toolchain, but it took foreverrrrr

RUN git clone https://github.com/aws/karpenter.git /karpenter
WORKDIR /karpenter
RUN GOPRIVATE=* go mod tidy
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't set GOPRIVATE=* since this causes go to skip checksum verifications... Do you just want the proxy off or is there more you're trying to do here?

4 changes: 2 additions & 2 deletions test/tools/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ Tools is an image that contains binaries useful for testing karpenter.

```
aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws
docker build ./test/tools -t public.ecr.aws/karpenter-testing/tools:latest
docker push public.ecr.aws/karpenter-testing/tools:latest
docker build ./test/tools --progress plain -t public.ecr.aws/karpenter-testing/tools:latest
docker push public.ecr.aws/karpenter-testing/tools:latest
```