From e123aa7cae5fc0da64a23e4d1e734e395e91b68a Mon Sep 17 00:00:00 2001 From: Matthew Long <61910737+thatmattlong@users.noreply.github.com> Date: Wed, 26 Jul 2023 09:52:27 -0700 Subject: [PATCH 1/2] fix: assume invalid semver CNI has the required dump state command (#2078) --- cni/client/client.go | 10 +++++++++- cns/cnireconciler/version.go | 10 +++++++++- cns/cnireconciler/version_test.go | 12 ++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/cni/client/client.go b/cni/client/client.go index 69e1bf4196..7c44f6e7cc 100644 --- a/cni/client/client.go +++ b/cni/client/client.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/platform" semver "github.com/hashicorp/go-version" + "github.com/pkg/errors" utilexec "k8s.io/utils/exec" ) @@ -18,6 +19,8 @@ type client struct { exec utilexec.Interface } +var ErrSemVerParse = errors.New("error parsing version") + func New(exec utilexec.Interface) *client { return &client{exec: exec} } @@ -58,5 +61,10 @@ func (c *client) GetVersion() (*semver.Version, error) { return nil, fmt.Errorf("Unexpected Azure CNI Version formatting: %v", output) } - return semver.NewVersion(res[3]) + version, versionErr := semver.NewVersion(res[3]) + if versionErr != nil { + return nil, errors.Wrap(ErrSemVerParse, versionErr.Error()) + } + + return version, nil } diff --git a/cns/cnireconciler/version.go b/cns/cnireconciler/version.go index 91a4667164..c6c6ccd27a 100644 --- a/cns/cnireconciler/version.go +++ b/cns/cnireconciler/version.go @@ -5,6 +5,7 @@ import ( "github.com/Azure/azure-container-networking/cni/client" semver "github.com/hashicorp/go-version" + "github.com/pkg/errors" "k8s.io/utils/exec" ) @@ -15,7 +16,9 @@ const lastCNIWithoutDumpStateVer = "1.4.6" // IsDumpStateVer checks if the CNI executable is a version that // has the dump state command required to initialize CNS from CNI // state and returns the result of that test or an error. Will always -// return false when there is an error. +// return false when there is an error unless the error was caused +// by the CNI not being a semver, in which case we'll assume we can +// use the command. func IsDumpStateVer() (bool, error) { return isDumpStateVer(exec.New()) } @@ -28,6 +31,11 @@ func isDumpStateVer(exec exec.Interface) (bool, error) { cnicli := client.New(exec) ver, err := cnicli.GetVersion() if err != nil { + // If the error was that the CNI isn't a valid semver, assume we have the + // the dump state command + if errors.Is(err, client.ErrSemVerParse) { + return true, nil + } return false, fmt.Errorf("failed to invoke CNI client.GetVersion(): %w", err) } return ver.GreaterThan(needVer), nil diff --git a/cns/cnireconciler/version_test.go b/cns/cnireconciler/version_test.go index eafe75b88a..aa04319650 100644 --- a/cns/cnireconciler/version_test.go +++ b/cns/cnireconciler/version_test.go @@ -48,6 +48,18 @@ func TestIsDumpStateVer(t *testing.T) { want: true, wantErr: false, }, + { + name: "non-semver", + exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.35_Win2019OverlayFix`), + want: true, + wantErr: false, + }, + { + name: "non-semver hotfix ver", + exec: newCNIVersionFakeExec(`Azure CNI Version v1.4.44.4`), + want: true, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 9dc9503e204c9440afd36a038ecdad3995049706 Mon Sep 17 00:00:00 2001 From: Vipul Singh Date: Tue, 13 Jun 2023 18:17:24 -0700 Subject: [PATCH 2/2] fix: Updating the vmsize for e2e cilium to avoid resource scarcity (#2014) CI: Testing the e2e test for cilium --- .../singletenancy/cilium/cilium-e2e-step-template.yaml | 5 ++++- .../singletenancy/overlay/overlay-e2e-step-template.yaml | 5 ++++- test/integration/utils_test.go | 9 +++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.pipelines/singletenancy/cilium/cilium-e2e-step-template.yaml b/.pipelines/singletenancy/cilium/cilium-e2e-step-template.yaml index eb7d80e016..21512932ae 100644 --- a/.pipelines/singletenancy/cilium/cilium-e2e-step-template.yaml +++ b/.pipelines/singletenancy/cilium/cilium-e2e-step-template.yaml @@ -32,7 +32,7 @@ steps: mkdir -p ~/.kube/ echo "Create AKS cluster" make -C ./hack/swift azcfg AZCLI=az REGION=$(REGION_AKS_CLUSTER_TEST) - make -C ./hack/swift byocni-up AZCLI=az REGION=$(REGION_AKS_CLUSTER_TEST) SUB=$(SUB_AZURE_NETWORK_AGENT_TEST) CLUSTER=${{ parameters.clusterName }}-$(make revision) + make -C ./hack/swift byocni-up AZCLI=az REGION=$(REGION_AKS_CLUSTER_TEST) SUB=$(SUB_AZURE_NETWORK_AGENT_TEST) CLUSTER=${{ parameters.clusterName }}-$(make revision) VM_SIZE=Standard_B2ms echo "Cluster successfully created" displayName: Create test cluster condition: succeeded() @@ -92,6 +92,9 @@ steps: displayName: "Run Azilium E2E" - script: | + echo "Status of the nodes and pods after the test" + kubectl get nodes -o wide + kubectl get pods -A -o wide echo "Logs will be available as a build artifact" ARTIFACT_DIR=$(Build.ArtifactStagingDirectory)/test-output/ echo $ARTIFACT_DIR diff --git a/.pipelines/singletenancy/overlay/overlay-e2e-step-template.yaml b/.pipelines/singletenancy/overlay/overlay-e2e-step-template.yaml index 074e0992b4..5a2f3e89c1 100644 --- a/.pipelines/singletenancy/overlay/overlay-e2e-step-template.yaml +++ b/.pipelines/singletenancy/overlay/overlay-e2e-step-template.yaml @@ -32,7 +32,7 @@ steps: mkdir -p ~/.kube/ echo "Create AKS Overlay cluster" make -C ./hack/swift azcfg AZCLI=az REGION=$(REGION_OVERLAY_CLUSTER_TEST) - make -C ./hack/swift overlay-byocni-up AZCLI=az REGION=$(REGION_OVERLAY_CLUSTER_TEST) SUB=$(SUB_AZURE_NETWORK_AGENT_TEST) CLUSTER=${{ parameters.clusterName }}-$(make revision) + make -C ./hack/swift overlay-byocni-up AZCLI=az REGION=$(REGION_OVERLAY_CLUSTER_TEST) SUB=$(SUB_AZURE_NETWORK_AGENT_TEST) CLUSTER=${{ parameters.clusterName }}-$(make revision) VM_SIZE=Standard_B2ms echo "Cluster successfully created" displayName: Create Overlay cluster condition: succeeded() @@ -98,6 +98,9 @@ steps: displayName: "Run Azilium E2E on AKS Overlay" - script: | + echo "Status of the nodes and pods after the test" + kubectl get nodes -o wide + kubectl get pods -A -o wide echo "Logs will be available as a build artifact" ARTIFACT_DIR=$(Build.ArtifactStagingDirectory)/test-output/ echo $ARTIFACT_DIR diff --git a/test/integration/utils_test.go b/test/integration/utils_test.go index 857180850f..cf79f7cab1 100644 --- a/test/integration/utils_test.go +++ b/test/integration/utils_test.go @@ -5,15 +5,16 @@ package k8s import ( "bytes" "context" - "errors" "io" "log" + "os" "strings" + "testing" "time" + "github.com/pkg/errors" + // crd "dnc/requestcontroller/kubernetes" - "os" - "testing" "github.com/Azure/azure-container-networking/test/integration/retry" apiv1 "k8s.io/api/core/v1" @@ -215,7 +216,7 @@ func waitForPodsRunning(ctx context.Context, clientset *kubernetes.Clientset, na for _, pod := range podList.Items { if pod.Status.PodIP == "" { - return errors.New("a pod has not been allocated an IP") + return errors.Wrapf(err, "Pod %s/%s has not been allocated an IP yet with reason %s", pod.Namespace, pod.Name, pod.Status.Message) } }