From b4ee39888bd00c6708989e1352d2ca958d74abd2 Mon Sep 17 00:00:00 2001 From: vishesh92 Date: Mon, 31 Aug 2026 13:53:02 +0530 Subject: [PATCH] Add a simulated dev/test environment and simulator-based e2e CI The repository had no way to exercise the CCM end to end. The only "run against real CloudStack" hook was configFromEnv() in cloudstack_test.go, which skips unless CS_API_URL and friends are set, and nothing set them. As a result EnsureLoadBalancer, UpdateLoadBalancer and EnsureLoadBalancerDeleted -- the three functions holding nearly all of the load balancer branching -- had no test coverage at all, and the README pointed at a Docker Hub image (cloudstack/simulator) that no longer exists. Add hack/e2e, which brings up a CloudStack simulator, deploys its advanced zone, mints admin API keys, creates a kind cluster and deploys CloudStack VMs matching its nodes, then runs the CCM against both. CloudStack calls go through cmk, so the scripts run the same commands the documentation tells you to run, and cmk's own async job handling removes any need to poll queryAsyncJobResult. docs/development.md walks through the same steps by hand so the environment is understandable rather than magic. Add a Go e2e suite under test/e2e covering load balancer lifecycle, node initialization, service annotations and the VPC/network ACL path. It is behind the e2e build tag, so it stays out of `make test` and `go build ./...`, and it needs no new module dependencies. Run all of it in CI as a matrix of the latest two Kubernetes minors against CloudStack 4.22.1.0 and 4.20.2.0. The CloudStack axis is not only version coverage: 4.22 and later update a load balancer rule's CIDR list in place while earlier releases delete and recreate the rule, so both branches are exercised. Cells run in parallel and share a single image build, and the simulator and kind node images are cached between runs, so the workflow costs about as much wall-clock as a single run. Fix several latent bugs in the load balancer path that the new suite exposed. Three call sites fetched CloudStack resources without the configured project: updateNetworkACL (the network, its ACL list and the ACL rule listing), getNetworkIDFromIPAddress (the public IP and its network), and the disassociation check in EnsureLoadBalancerDeleted. On a VPC owned by a project this made every LoadBalancer service fail with "error fetching Network with ID" and never get an ingress address, and it leaked the public IP on deletion. getNetworkIDFromIPAddress also reported a failed network lookup as success by returning the wrong error variable, and guarded on Networkid while looking up Associatednetworkid; either could hand the caller an empty network ID, which GetNetworkByID does not reject but looks up as an unfiltered network list, so it could resolve to an arbitrary network instead of failing. Fix a load balancer rule leak. CloudStack does not enforce unique rule names, but loadBalancer.rules is keyed by name, so a duplicate silently displaced its twin in the map and then survived EnsureLoadBalancerDeleted with no service left to reference it. Duplicates are now tracked separately and removed on both reconcile and delete, together with the firewall rule and public IP that only the duplicate used, leaving the network ACL rules it shares with the kept rule in place. The rule that survives is the one on the address the service is published on -- spec.loadBalancerIP when set, otherwise the IP already in status.loadBalancer.ingress -- so the sweep cannot delete the rule clients are pointing at. On the delete path a failed sweep is logged rather than returned, so a duplicate that cannot be removed leaks a rule instead of holding the service in Terminating for ever. Harden getManagementServerVersion, which sliced the version string to three components without checking its length. A management server reporting fewer than three, such as "4.22" or a bare "24" under CloudStack's new versioning scheme, panicked the controller at startup. Also add the local cloud-config, cmk-config and kube-config files to .gitignore. They hold live credentials and were previously untracked but not ignored. Fixes #4 Co-Authored-By: Claude Opus 5 --- .github/workflows/e2e-simulator.yml | 182 ++++++++++ .gitignore | 8 +- .golangci.yml | 3 + Makefile | 45 ++- README.md | 65 ++-- cloudstack.go | 3 +- cloudstack_loadbalancer.go | 124 ++++++- cloudstack_loadbalancer_test.go | 339 ++++++++++++++++++- cloudstack_test.go | 45 +++ docs/development.md | 424 +++++++++++++++++++++++ hack/e2e/10-simulator-up.sh | 126 +++++++ hack/e2e/20-kind-up.sh | 52 +++ hack/e2e/30-topology-isolated.sh | 87 +++++ hack/e2e/40-ccm-deploy.sh | 98 ++++++ hack/e2e/50-topology-vpc.sh | 123 +++++++ hack/e2e/90-collect-artifacts.sh | 67 ++++ hack/e2e/99-down.sh | 38 +++ hack/e2e/env.sh | 71 ++++ hack/e2e/kind-config.yaml | 62 ++++ hack/e2e/lib/cmk.sh | 69 ++++ hack/e2e/lib/log.sh | 45 +++ hack/e2e/up.sh | 38 +++ test/e2e/annotations_test.go | 173 ++++++++++ test/e2e/framework.go | 506 ++++++++++++++++++++++++++++ test/e2e/loadbalancer_test.go | 208 ++++++++++++ test/e2e/node_test.go | 136 ++++++++ test/e2e/vpc_test.go | 261 ++++++++++++++ 27 files changed, 3328 insertions(+), 70 deletions(-) create mode 100644 .github/workflows/e2e-simulator.yml create mode 100644 docs/development.md create mode 100755 hack/e2e/10-simulator-up.sh create mode 100755 hack/e2e/20-kind-up.sh create mode 100755 hack/e2e/30-topology-isolated.sh create mode 100755 hack/e2e/40-ccm-deploy.sh create mode 100755 hack/e2e/50-topology-vpc.sh create mode 100755 hack/e2e/90-collect-artifacts.sh create mode 100755 hack/e2e/99-down.sh create mode 100755 hack/e2e/env.sh create mode 100644 hack/e2e/kind-config.yaml create mode 100644 hack/e2e/lib/cmk.sh create mode 100644 hack/e2e/lib/log.sh create mode 100755 hack/e2e/up.sh create mode 100644 test/e2e/annotations_test.go create mode 100644 test/e2e/framework.go create mode 100644 test/e2e/loadbalancer_test.go create mode 100644 test/e2e/node_test.go create mode 100644 test/e2e/vpc_test.go diff --git a/.github/workflows/e2e-simulator.yml b/.github/workflows/e2e-simulator.yml new file mode 100644 index 00000000..908747b3 --- /dev/null +++ b/.github/workflows/e2e-simulator.yml @@ -0,0 +1,182 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: E2E (CloudStack Simulator) + +on: + push: + branches: + - main + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + # Build the CCM image once and share it with every matrix cell. The + # distroless image is small enough that passing it as an artifact is much + # cheaper than four redundant builds. + build: + if: github.repository == 'apache/cloudstack-kubernetes-provider' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v6 + + - uses: docker/setup-buildx-action@v3 + + - name: Build CCM image + uses: docker/build-push-action@v6 + with: + context: . + load: true + platforms: linux/amd64 + tags: apache/cloudstack-kubernetes-provider:e2e + cache-from: type=gha + cache-to: type=gha,mode=max + + - name: Export image + run: docker save apache/cloudstack-kubernetes-provider:e2e | zstd -T0 -o ccm-image.tar.zst + + - uses: actions/upload-artifact@v4 + with: + name: ccm-image + path: ccm-image.tar.zst + retention-days: 1 + + e2e: + needs: build + if: github.repository == 'apache/cloudstack-kubernetes-provider' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + k8s: ['v1.37.0', 'v1.36.4'] + acs: ['4.22.1.0', '4.20.2.0'] + env: + KIND_NODE_IMAGE: kindest/node:${{ matrix.k8s }} + SIM_TAG: ${{ matrix.acs }} + CCM_IMAGE: apache/cloudstack-kubernetes-provider:e2e + CMK_VERSION: "6.5.0" + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + # Released image tags never change, so cache them per version rather than + # pulling on every run. Keyed separately so the ~2 GB simulator tarball is + # shared across both Kubernetes versions instead of once per matrix cell. + # `docker save` output is already compressed (Docker keeps the layer + # blobs), so these are ~2 GB and ~370 MB respectively. + - name: Restore simulator image + uses: actions/cache@v4 + with: + path: /tmp/images/simulator.tar + key: image-simulator-${{ matrix.acs }} + + - name: Restore kind node image + uses: actions/cache@v4 + with: + path: /tmp/images/kind-node.tar + key: image-kind-node-${{ matrix.k8s }} + + - name: Load images + run: | + mkdir -p /tmp/images + load_or_pull() { + if [ -f "$1" ]; then + docker load -i "$1" + else + docker pull "$2" + docker save "$2" -o "$1" + fi + } + load_or_pull /tmp/images/simulator.tar "apache/cloudstack-simulator:${SIM_TAG}" + load_or_pull /tmp/images/kind-node.tar "${KIND_NODE_IMAGE}" + + - uses: actions/download-artifact@v4 + with: + name: ccm-image + + - name: Load CCM image + run: | + zstd -dc ccm-image.tar.zst | docker load + echo "CCM_IMAGE_PREBUILT=true" >>"$GITHUB_ENV" + + - uses: helm/kind-action@v1 + with: + install_only: true + version: v0.32.0 + + - name: Install cmk (CloudMonkey) + run: | + curl -fsSL -o /usr/local/bin/cmk \ + "https://github.com/apache/cloudstack-cloudmonkey/releases/download/${CMK_VERSION}/cmk.linux.x86-64" + chmod +x /usr/local/bin/cmk + cmk version + + - name: Start simulator and deploy zone + run: hack/e2e/10-simulator-up.sh + + - name: Create kind cluster + run: hack/e2e/20-kind-up.sh + + - name: Create isolated network topology + run: hack/e2e/30-topology-isolated.sh + + - name: Deploy the cloud controller manager + run: hack/e2e/40-ccm-deploy.sh + + # These go through the make targets rather than repeating the commands, + # so CI and a local run cannot drift apart -- in particular the API + # endpoint is derived from SIM_HOST_PORT in one place. + - name: Unit and acceptance tests against the live simulator + run: | + . hack/e2e/_out/keys.env + make test + + - name: E2E phase 1 (load balancer, nodes, annotations) + run: make test-e2e + + - name: Create VPC topology + run: make e2e-vpc + + - name: E2E phase 2 (VPC / network ACL) + run: make test-e2e-vpc + + - name: Collect artifacts + if: always() + run: hack/e2e/90-collect-artifacts.sh + + - uses: actions/upload-artifact@v4 + if: always() + with: + name: e2e-artifacts-${{ matrix.k8s }}-${{ matrix.acs }} + path: hack/e2e/_out/artifacts + retention-days: 7 + + - name: Tear down + if: always() + run: hack/e2e/99-down.sh diff --git a/.gitignore b/.gitignore index 79e12562..d07d962c 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,12 @@ go.work.sum # env file .env +# Local dev/test credentials and generated harness state — never commit these +/cloud-config +/cmk-config +/kube-config +/hack/e2e/_out/ + # Editor/IDE .idea/ -.vscode/ \ No newline at end of file +.vscode/ diff --git a/.golangci.yml b/.golangci.yml index 379c873f..84ae959e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,6 +19,9 @@ version: "2" run: modules-download-mode: readonly issues-exit-code: 1 + # Lint the build-tagged e2e suite too; without this, goheader/gosec silently skip it. + build-tags: + - e2e linters: enable: - goheader diff --git a/Makefile b/Makefile index bbb03f0d..6abed19a 100644 --- a/Makefile +++ b/Makefile @@ -29,10 +29,21 @@ LDFLAGS="-X k8s.io/kubernetes/pkg/version.gitVersion=${GIT_VERSION} -X k8s.io/ku export CGO_ENABLED=0 export GO111MODULE=on +# Keep these in step with hack/e2e/env.sh: both are overridable from the +# environment, so `make test-e2e` reaches the same simulator `make e2e-up` +# published rather than assuming the default port. +SIM_HOST_PORT ?= 8080 +CS_API_URL ?= http://localhost:$(SIM_HOST_PORT)/client/api +# Exported so the harness scripts and the acceptance tests in `make test` -- +# which read CS_API_URL from the environment -- agree with the targets here +# without every caller having to repeat the endpoint. +export SIM_HOST_PORT +export CS_API_URL + CMD_SRC=\ cmd/cloudstack-ccm/main.go -.PHONY: all clean docker +.PHONY: all clean docker e2e-up e2e-down e2e-vpc test-e2e test-e2e-vpc all: cloudstack-ccm @@ -53,6 +64,38 @@ ifneq (${GIT_IS_TAG},NOT_A_TAG) docker tag apache/cloudstack-kubernetes-provider:${GIT_COMMIT_SHORT} apache/cloudstack-kubernetes-provider:${GIT_TAG} endif +# Simulator-based e2e environment; see docs/development.md +e2e-up: + hack/e2e/up.sh + +e2e-down: + hack/e2e/99-down.sh + +# go test runs with the package directory as its working directory, so +# KUBECONFIG must be absolute. +test-e2e: + @test -f hack/e2e/_out/keys.env || (echo "environment not up; run 'make e2e-up' first" && exit 1) + . hack/e2e/_out/keys.env && \ + KUBECONFIG=${CURDIR}/hack/e2e/_out/kubeconfig \ + CS_API_URL=$(CS_API_URL) \ + go test -tags e2e -v -timeout 30m ./test/e2e/... -run 'TestLB|TestNode|TestAnnot' + +# Phase 2. Run hack/e2e/50-topology-vpc.sh first: it creates the project, VPC +# and tier, and re-points the CCM at the project. These tests only exercise +# anything with CS_PROJECT_ID set, and skip otherwise. +e2e-vpc: + hack/e2e/50-topology-vpc.sh + +test-e2e-vpc: + @test -f hack/e2e/_out/ids.env || (echo "VPC topology not created; run 'make e2e-vpc' first" && exit 1) + @grep -q E2E_PROJECT_ID hack/e2e/_out/ids.env || (echo "VPC topology not created; run 'make e2e-vpc' first" && exit 1) + . hack/e2e/_out/keys.env && . hack/e2e/_out/ids.env && \ + KUBECONFIG=${CURDIR}/hack/e2e/_out/kubeconfig \ + CS_API_URL=$(CS_API_URL) \ + CS_PROJECT_ID="$$E2E_PROJECT_ID" \ + E2E_ACL_ID="$$E2E_ACL_ID" E2E_VPC_ID="$$E2E_VPC_ID" \ + go test -tags e2e -v -timeout 30m ./test/e2e/... -run 'TestVPC' + lint: gofmt @(echo "Running golangci-lint...") golangci-lint run diff --git a/README.md b/README.md index fc4922b7..9d2d93a8 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,11 @@ explicitly set `region` in that case. The access token needs to be able to fetch VM information and deploy load balancers in the project or domain where the nodes reside. +The account must also be allowed to call `listManagementServersMetrics`, which the controller uses +on startup to determine the management server version. This is a root admin API and is **not** +included in the default `User` role; without it the controller exits immediately with +`no management servers found`. + To create the secret, use the following command: ```bash kubectl -n kube-system create secret generic cloudstack-secret --from-file=cloud-config @@ -423,9 +428,15 @@ make docker ### Testing -You need a local instance of the CloudStack Management Server or a 'real' one to connect to. +Unit tests need nothing but Go: + +```bash +make test +``` + +For anything beyond that you need a CloudStack Management Server to talk to. The CCM supports the same cloud-config configuration file format used by [the cs tool](https://github.com/exoscale/cs), -so you can simply point it to that. +so you can simply point it at one you already have: ```bash ./cloudstack-ccm --cloud-provider external-cloudstack --cloud-config ./cloud-config --kubeconfig ~/.kube/config @@ -434,45 +445,21 @@ so you can simply point it to that. Point `--kubeconfig` at a kubeconfig for your Kubernetes development cluster, and `--cloud-config` at a `cloud-config` for the CloudStack installation you want to talk to. -If you don't have a 'real' CloudStack installation, you can also launch a local [simulator instance](https://hub.docker.com/r/cloudstack/simulator) instead. This is very useful for dry-run testing. - -### Debugging - -You can use the VSCode extension [Go](https://marketplace.visualstudio.com/items?itemName=golang.go) to debug the CCM. -Add the following configuration to the `.vscode/launch.json` file to launch the CCM and debug it. - -```json -{ - "version": "0.2.0", - "configurations": [ - { - "name": "Launch CloudStack CCM", - "type": "go", - "request": "launch", - "mode": "auto", - "program": "${workspaceFolder}/cmd/cloudstack-ccm", - "env": {}, - "args": [ - "--cloud-provider=external-cloudstack", - "--cloud-config=${workspaceFolder}/cloud-config", - "--kubeconfig=${env:HOME}/.kube/config", - "--leader-elect=false", - "--v=4" - ], - "showLog": true, - "trace": "verbose" - }, - { - "name": "Attach to Process", - "type": "go", - "request": "attach", - "mode": "local", - "processId": 0 - } - ] -} +If you don't have a 'real' CloudStack installation, you don't need one. The repository ships a +fully simulated environment — a kind cluster, the +[CloudStack simulator](https://hub.docker.com/r/apache/cloudstack-simulator) and the CCM, all in +containers: + +```bash +make e2e-up # bring the environment up +make test-e2e # run the end-to-end suite against it +make e2e-down # tear it down ``` +See [docs/development.md](docs/development.md) for the full walkthrough, how to run the CCM as a +host process under a debugger, the VPC scenario (`make e2e-vpc` / `make test-e2e-vpc`), and +troubleshooting. + ## Copyright Copyright 2019 The Apache Software Foundation diff --git a/cloudstack.go b/cloudstack.go index 2294de6f..e7224207 100644 --- a/cloudstack.go +++ b/cloudstack.go @@ -128,7 +128,8 @@ func (cs *CSCloud) getManagementServerVersion() (semver.Version, error) { return semver.Version{}, errors.New("no management servers found") } version := msServersResp.ManagementServersMetrics[0].Version - v, err := semver.ParseTolerant(strings.Join(strings.Split(version, ".")[0:3], ".")) + parts := strings.Split(version, ".") + v, err := semver.ParseTolerant(strings.Join(parts[:min(len(parts), 3)], ".")) if err != nil { klog.Errorf("failed to parse management server version: %v", err) return semver.Version{}, err diff --git a/cloudstack_loadbalancer.go b/cloudstack_loadbalancer.go index ffbdd7cd..69d16f51 100644 --- a/cloudstack_loadbalancer.go +++ b/cloudstack_loadbalancer.go @@ -69,6 +69,7 @@ type loadBalancer struct { networkID string projectID string rules map[string]*cloudstack.LoadBalancerRule + duplicateRules []*cloudstack.LoadBalancerRule ipAssociatedByController bool } @@ -109,6 +110,10 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s return nil, err } + if err := lb.deleteDuplicateRules(); err != nil { + return nil, err + } + // Set the load balancer algorithm. switch service.Spec.SessionAffinity { case corev1.ServiceAffinityNone: @@ -331,6 +336,13 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st return err } + // Reported only once this service's own resources are gone, so a retry sees the + // leftover duplicate as an ordinary rule and deletes it through the path above. + sweepErr := lb.deleteDuplicateRules() + if sweepErr != nil { + klog.Errorf("Error removing duplicate load balancer rules for %v/%v: %v", service.Namespace, service.Name, sweepErr) + } + for _, lbRule := range lb.rules { klog.V(4).Infof("Deleting firewall rules / Network ACLs for load balancer: %v", lbRule.Name) protocol := ProtocolFromLoadBalancer(lbRule.Protocol) @@ -391,7 +403,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st // Annotation is set, so check if there are any other load balancer rules using this IP. // Since we've already deleted all rules for this service, any remaining rules must belong // to other services. If no other rules exist, it's safe to disassociate the IP. - ip, count, err := lb.Address.GetPublicIpAddressByID(lb.ipAddrID) + ip, count, err := lb.Address.GetPublicIpAddressByID(lb.ipAddrID, cloudstack.WithProject(lb.projectID)) if err != nil { klog.Errorf("Error retrieving IP address %v for disassociation check: %v", lb.ipAddr, err) shouldDisassociate = false @@ -423,7 +435,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st } } - return nil + return sweepErr } // GetLoadBalancerName retrieves the name of the LoadBalancer. @@ -453,7 +465,25 @@ func (cs *CSCloud) getLoadBalancer(service *corev1.Service) (*loadBalancer, erro return nil, fmt.Errorf("error retrieving load balancer rules: %v", err) } + // Keeping the rule on the address the Service is already published on stops a + // duplicate sweep from deleting the rule that clients and DNS are pointing at. + preferredIP := service.Spec.LoadBalancerIP + if preferredIP == "" && len(service.Status.LoadBalancer.Ingress) > 0 { + preferredIP = service.Status.LoadBalancer.Ingress[0].IP + } + for _, lbRule := range l.LoadBalancerRules { + if existing, seen := lb.rules[lbRule.Name]; seen { + duplicate := lbRule + if lbRule.Publicip == preferredIP && existing.Publicip != preferredIP { + duplicate = existing + } + klog.Warningf("Duplicate load balancer rule %v for service %v/%v, removing %v on %v", lbRule.Name, service.Namespace, service.Name, duplicate.Id, duplicate.Publicip) + lb.duplicateRules = append(lb.duplicateRules, duplicate) + if duplicate == lbRule { + continue + } + } lb.rules[lbRule.Name] = lbRule if lb.ipAddr != "" && lb.ipAddr != lbRule.Publicip { @@ -470,24 +500,26 @@ func (cs *CSCloud) getLoadBalancer(service *corev1.Service) (*loadBalancer, erro } // Get network ID from Public IP Address +// Every failure returns an error: GetNetworkByID does not reject an empty ID but +// matches an unfiltered network list, so ("", nil) would resolve to any network. func (cs *CSCloud) getNetworkIDFromIPAddress(publicIpId string) (string, error) { - ip, count, err := cs.client.Address.GetPublicIpAddressByID(publicIpId) + ip, count, err := cs.client.Address.GetPublicIpAddressByID(publicIpId, cloudstack.WithProject(cs.projectID)) if err != nil { klog.Errorf("Failed to fetch the public IP for id: %v", publicIpId) return "", err } if count == 0 { - return "", err + return "", fmt.Errorf("no public IP address found with ID %v", publicIpId) } - if ip.Networkid != "" { - network, _, netErr := cs.client.Network.GetNetworkByID(ip.Associatednetworkid) + if ip.Associatednetworkid != "" { + network, _, netErr := cs.client.Network.GetNetworkByID(ip.Associatednetworkid, cloudstack.WithProject(cs.projectID)) if netErr != nil { klog.Errorf("Failed to fetch the network for id: %v", ip.Associatednetworkid) - return "", err + return "", netErr } return network.Id, nil } - return "", nil + return "", fmt.Errorf("public IP address %v is not associated with a network", publicIpId) } // verifyHosts verifies if all hosts belong to the same network, and returns the host ID's and network ID. @@ -770,12 +802,76 @@ func (lb *loadBalancer) deleteLoadBalancerRule(lbRule *cloudstack.LoadBalancerRu return fmt.Errorf("error deleting load balancer rule %v: %v", lbRule.Name, err) } - // Delete the rule from the map as it no longer exists - delete(lb.rules, lbRule.Name) + // A duplicate shares its name with the rule being kept, which owns the map entry. + if kept, ok := lb.rules[lbRule.Name]; ok && kept.Id == lbRule.Id { + delete(lb.rules, lbRule.Name) + } return nil } +// deleteDuplicateRules removes rules that collided by name with the one being +// managed. A duplicate sits on its own public IP, since CloudStack rejects a +// second rule on the same IP and port, so its firewall rule and IP go with it; +// network ACL rules are shared per tier and port with the kept rule and stay. +func (lb *loadBalancer) deleteDuplicateRules() error { + for _, lbRule := range lb.duplicateRules { + klog.V(4).Infof("Deleting duplicate load balancer rule: %v (%v)", lbRule.Name, lbRule.Id) + if err := lb.deleteDuplicateRule(lbRule); err != nil { + return err + } + } + lb.duplicateRules = nil + + return nil +} + +// deleteDuplicateRule tears down one duplicate: its firewall rule, the rule +// itself, and its public IP once no other rule uses that IP. +func (lb *loadBalancer) deleteDuplicateRule(lbRule *cloudstack.LoadBalancerRule) error { + port, err := strconv.Atoi(lbRule.Publicport) + protocol := ProtocolFromLoadBalancer(lbRule.Protocol) + if err != nil || protocol == LoadBalancerProtocolInvalid { + klog.Warningf("Leaving duplicate rule %v (%v) in place: unusable public port %q or protocol %q", lbRule.Name, lbRule.Id, lbRule.Publicport, lbRule.Protocol) + return nil + } + if _, err := lb.deleteFirewallRule(lbRule.Publicipid, port, protocol); err != nil { + return err + } + if err := lb.deleteLoadBalancerRule(lbRule); err != nil { + return err + } + if lbRule.Publicipid == lb.ipAddrID { + return nil + } + inUse, err := lb.publicIPHasRules(lbRule.Publicipid) + if err != nil || inUse { + return err + } + + p := lb.Address.NewDisassociateIpAddressParams(lbRule.Publicipid) + if _, err := lb.Address.DisassociateIpAddress(p); err != nil { + return fmt.Errorf("error releasing public IP %v: %v", lbRule.Publicipid, err) + } + + return nil +} + +// publicIPHasRules reports whether any load balancer rule still uses the IP. +func (lb *loadBalancer) publicIPHasRules(publicIPID string) (bool, error) { + p := lb.LoadBalancer.NewListLoadBalancerRulesParams() + p.SetPublicipid(publicIPID) + p.SetListall(true) + if lb.projectID != "" { + p.SetProjectid(lb.projectID) + } + rules, err := lb.LoadBalancer.ListLoadBalancerRules(p) + if err != nil { + return false, fmt.Errorf("error listing load balancer rules on IP %v: %v", publicIPID, err) + } + return rules.Count > 0, nil +} + // assignHostsToRule assigns hosts to a load balancer rule. func (lb *loadBalancer) assignHostsToRule(lbRule *cloudstack.LoadBalancerRule, hostIDs []string) error { p := lb.LoadBalancer.NewAssignToLoadBalancerRuleParams(lbRule.Id) @@ -981,12 +1077,12 @@ func (lb *loadBalancer) updateFirewallRule(publicIpId string, publicPort int, pr } func (lb *loadBalancer) updateNetworkACL(publicPort int, protocol LoadBalancerProtocol, networkId string) (bool, error) { - network, _, err := lb.Network.GetNetworkByID(networkId) + network, _, err := lb.Network.GetNetworkByID(networkId, cloudstack.WithProject(lb.projectID)) if err != nil { return false, fmt.Errorf("error fetching Network with ID: %v, due to: %s", networkId, err) } - networkAclList, count, err := lb.NetworkACL.GetNetworkACLListByID(network.Aclid) + networkAclList, count, err := lb.NetworkACL.GetNetworkACLListByID(network.Aclid, cloudstack.WithProject(lb.projectID)) if err != nil { return false, fmt.Errorf("error fetching Network ACL List with ID: %v, due to: %s", network.Aclid, err) } @@ -1003,6 +1099,10 @@ func (lb *loadBalancer) updateNetworkACL(publicPort int, protocol LoadBalancerPr networkAclParams := lb.NetworkACL.NewListNetworkACLsParams() networkAclParams.SetAclid(network.Aclid) networkAclParams.SetNetworkid(networkId) + networkAclParams.SetListall(true) + if lb.projectID != "" { + networkAclParams.SetProjectid(lb.projectID) + } networkAclResponse, err := lb.NetworkACL.ListNetworkACLs(networkAclParams) diff --git a/cloudstack_loadbalancer_test.go b/cloudstack_loadbalancer_test.go index 4bbf38e7..39b229d5 100644 --- a/cloudstack_loadbalancer_test.go +++ b/cloudstack_loadbalancer_test.go @@ -20,6 +20,7 @@ package cloudstack import ( + "context" "fmt" "reflect" "sort" @@ -2829,8 +2830,8 @@ func TestUpdateNetworkACL(t *testing.T) { } gomock.InOrder( - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), - mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456").Return(aclListResp, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), + mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456", gomock.Any()).Return(aclListResp, 1, nil), mockNetworkACL.EXPECT().NewListNetworkACLsParams().Return(listParams), mockNetworkACL.EXPECT().ListNetworkACLs(gomock.Any()).Return(listResp, nil), mockNetworkACL.EXPECT().NewCreateNetworkACLParams("tcp").Return(createParams), @@ -2884,8 +2885,8 @@ func TestUpdateNetworkACL(t *testing.T) { } gomock.InOrder( - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), - mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456").Return(aclListResp, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), + mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456", gomock.Any()).Return(aclListResp, 1, nil), mockNetworkACL.EXPECT().NewListNetworkACLsParams().Return(listParams), mockNetworkACL.EXPECT().ListNetworkACLs(gomock.Any()).Return(listResp, nil), ) @@ -2924,8 +2925,8 @@ func TestUpdateNetworkACL(t *testing.T) { } gomock.InOrder( - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), - mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456").Return(aclListResp, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), + mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456", gomock.Any()).Return(aclListResp, 1, nil), ) lb := &loadBalancer{ @@ -2951,7 +2952,7 @@ func TestUpdateNetworkACL(t *testing.T) { mockNetwork := cloudstack.NewMockNetworkServiceIface(ctrl) apiErr := fmt.Errorf("network API error") - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(nil, 1, apiErr) + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(nil, 1, apiErr) lb := &loadBalancer{ CloudStackClient: &cloudstack.CloudStackClient{ @@ -2983,8 +2984,8 @@ func TestUpdateNetworkACL(t *testing.T) { apiErr := fmt.Errorf("ACL list API error") gomock.InOrder( - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), - mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456").Return(nil, 0, apiErr), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), + mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456", gomock.Any()).Return(nil, 0, apiErr), ) lb := &loadBalancer{ @@ -3024,8 +3025,8 @@ func TestUpdateNetworkACL(t *testing.T) { apiErr := fmt.Errorf("list ACL API error") gomock.InOrder( - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), - mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456").Return(aclListResp, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), + mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456", gomock.Any()).Return(aclListResp, 1, nil), mockNetworkACL.EXPECT().NewListNetworkACLsParams().Return(listParams), mockNetworkACL.EXPECT().ListNetworkACLs(gomock.Any()).Return(nil, apiErr), ) @@ -3073,8 +3074,8 @@ func TestUpdateNetworkACL(t *testing.T) { apiErr := fmt.Errorf("create ACL API error") gomock.InOrder( - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), - mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456").Return(aclListResp, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), + mockNetworkACL.EXPECT().GetNetworkACLListByID("acl-456", gomock.Any()).Return(aclListResp, 1, nil), mockNetworkACL.EXPECT().NewListNetworkACLsParams().Return(listParams), mockNetworkACL.EXPECT().ListNetworkACLs(gomock.Any()).Return(listResp, nil), mockNetworkACL.EXPECT().NewCreateNetworkACLParams("tcp").Return(createParams), @@ -3385,6 +3386,127 @@ func TestGetLoadBalancer(t *testing.T) { t.Errorf("error message = %q, want to contain 'error retrieving load balancer rules'", err.Error()) } }) + + listDuplicates := func(t *testing.T, requestedIP, publishedIP string, rules ...*cloudstack.LoadBalancerRule) *loadBalancer { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + mockLB.EXPECT().NewListLoadBalancerRulesParams().Return(&cloudstack.ListLoadBalancerRulesParams{}) + mockLB.EXPECT().ListLoadBalancerRules(gomock.Any()). + Return(&cloudstack.ListLoadBalancerRulesResponse{Count: len(rules), LoadBalancerRules: rules}, nil) + + cs := &CSCloud{client: &cloudstack.CloudStackClient{LoadBalancer: mockLB}} + service := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "test-service", Namespace: "default"}, + Spec: corev1.ServiceSpec{LoadBalancerIP: requestedIP}, + } + if publishedIP != "" { + service.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{IP: publishedIP}} + } + + lb, err := cs.getLoadBalancer(service) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return lb + } + onAutoIP := &cloudstack.LoadBalancerRule{Id: "rule-auto", Name: "test-service-tcp-80", Publicip: "203.0.113.1", Publicipid: "ip-auto"} + onRequestedIP := &cloudstack.LoadBalancerRule{Id: "rule-requested", Name: "test-service-tcp-80", Publicip: "203.0.113.9", Publicipid: "ip-requested"} + + assertKept := func(t *testing.T, lb *loadBalancer, kept, discarded *cloudstack.LoadBalancerRule) { + if got := lb.rules["test-service-tcp-80"]; got != kept { + t.Errorf("kept rule = %v, want %v", got.Id, kept.Id) + } + if len(lb.duplicateRules) != 1 || lb.duplicateRules[0] != discarded { + t.Errorf("duplicateRules = %+v, want only %v", lb.duplicateRules, discarded.Id) + } + if lb.ipAddr != kept.Publicip || lb.ipAddrID != kept.Publicipid { + t.Errorf("ipAddr/ipAddrID = %v/%v, want the kept rule's %v/%v", lb.ipAddr, lb.ipAddrID, kept.Publicip, kept.Publicipid) + } + } + + t.Run("the first of two same-named rules is kept", func(t *testing.T) { + lb := listDuplicates(t, "", "", onAutoIP, onRequestedIP) + assertKept(t, lb, onAutoIP, onRequestedIP) + }) + + t.Run("a later rule on the requested IP is kept over an earlier one", func(t *testing.T) { + lb := listDuplicates(t, onRequestedIP.Publicip, "", onAutoIP, onRequestedIP) + assertKept(t, lb, onRequestedIP, onAutoIP) + }) + + t.Run("an earlier rule on the requested IP stays kept", func(t *testing.T) { + lb := listDuplicates(t, onRequestedIP.Publicip, "", onRequestedIP, onAutoIP) + assertKept(t, lb, onRequestedIP, onAutoIP) + }) + + t.Run("the rule on the published ingress IP is kept when no IP was requested", func(t *testing.T) { + lb := listDuplicates(t, "", onRequestedIP.Publicip, onAutoIP, onRequestedIP) + assertKept(t, lb, onRequestedIP, onAutoIP) + }) + + t.Run("a requested IP outranks the published ingress IP", func(t *testing.T) { + lb := listDuplicates(t, onAutoIP.Publicip, onRequestedIP.Publicip, onRequestedIP, onAutoIP) + assertKept(t, lb, onAutoIP, onRequestedIP) + }) +} + +// A failed sweep is reported so the service controller retries, but only after +// this service's own rules and IP are gone, so the retry sees the leftover +// duplicate as an ordinary rule instead of blocking deletion for ever. +func TestEnsureLoadBalancerDeletedReportsASweepFailureLast(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + mockFW := cloudstack.NewMockFirewallServiceIface(ctrl) + mockAddr := cloudstack.NewMockAddressServiceIface(ctrl) + mockNet := cloudstack.NewMockNetworkServiceIface(ctrl) + + kept := &cloudstack.LoadBalancerRule{Id: "keep", Name: "test-service-tcp-80", Publicip: "203.0.113.1", Publicipid: "ip-keep", Publicport: "80", Protocol: "tcp"} + duplicate := &cloudstack.LoadBalancerRule{Id: "dup", Name: "test-service-tcp-80", Publicip: "203.0.113.2", Publicipid: "ip-dup", Publicport: "80", Protocol: "tcp"} + + mockLB.EXPECT().NewListLoadBalancerRulesParams().Return(&cloudstack.ListLoadBalancerRulesParams{}) + mockLB.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(&cloudstack.ListLoadBalancerRulesResponse{ + Count: 2, LoadBalancerRules: []*cloudstack.LoadBalancerRule{kept, duplicate}, + }, nil) + + // The sweep fails while listing the duplicate's firewall rules. + mockFW.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}) + mockFW.EXPECT().ListFirewallRules(gomock.Any()).Return(nil, fmt.Errorf("firewall API down")) + + // Deletion of the kept rule must still happen. + mockAddr.EXPECT().GetPublicIpAddressByID("ip-keep", gomock.Any()). + Return(&cloudstack.PublicIpAddress{Id: "ip-keep", Associatednetworkid: "net-1"}, 1, nil) + // Once inside getNetworkIDFromIPAddress, once in the delete loop itself. + mockNet.EXPECT().GetNetworkByID("net-1", gomock.Any()). + Return(&cloudstack.Network{Id: "net-1"}, 1, nil).Times(2) + mockFW.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}) + mockFW.EXPECT().ListFirewallRules(gomock.Any()).Return(&cloudstack.ListFirewallRulesResponse{}, nil) + deleteParams := &cloudstack.DeleteLoadBalancerRuleParams{} + mockLB.EXPECT().NewDeleteLoadBalancerRuleParams("keep").Return(deleteParams) + mockLB.EXPECT().DeleteLoadBalancerRule(deleteParams).Return(&cloudstack.DeleteLoadBalancerRuleResponse{}, nil) + + // And the load balancer IP must still be released. + release := &cloudstack.DisassociateIpAddressParams{} + mockAddr.EXPECT().NewDisassociateIpAddressParams("ip-keep").Return(release) + mockAddr.EXPECT().DisassociateIpAddress(release).Return(&cloudstack.DisassociateIpAddressResponse{}, nil) + + cs := &CSCloud{client: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, Firewall: mockFW, Address: mockAddr, Network: mockNet, + }} + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service", Namespace: "default"}} + + err := cs.EnsureLoadBalancerDeleted(context.TODO(), "test", service) + if err == nil { + t.Fatal("expected the sweep failure to be reported so the delete is retried") + } + if !strings.Contains(err.Error(), "firewall API down") { + t.Errorf("error = %q, want it to carry the sweep failure", err) + } + // gomock asserts on Cleanup that the kept rule was deleted and the IP released + // despite the failure; without that the retry would have nothing to converge on. } func TestGetNetworkIDFromIPAddress(t *testing.T) { @@ -3407,8 +3529,8 @@ func TestGetNetworkIDFromIPAddress(t *testing.T) { } gomock.InOrder( - mockAddress.EXPECT().GetPublicIpAddressByID("ip-123").Return(ipResp, 1, nil), - mockNetwork.EXPECT().GetNetworkByID("net-123").Return(networkResp, 1, nil), + mockAddress.EXPECT().GetPublicIpAddressByID("ip-123", gomock.Any()).Return(ipResp, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(networkResp, 1, nil), ) cs := &CSCloud{ @@ -3434,7 +3556,7 @@ func TestGetNetworkIDFromIPAddress(t *testing.T) { mockAddress := cloudstack.NewMockAddressServiceIface(ctrl) apiErr := fmt.Errorf("IP not found") - mockAddress.EXPECT().GetPublicIpAddressByID("ip-123").Return(nil, 0, apiErr) + mockAddress.EXPECT().GetPublicIpAddressByID("ip-123", gomock.Any()).Return(nil, 0, apiErr) cs := &CSCloud{ client: &cloudstack.CloudStackClient{ @@ -3450,6 +3572,191 @@ func TestGetNetworkIDFromIPAddress(t *testing.T) { t.Errorf("error = %v, want %v", err, apiErr) } }) + + // The following two cases used to return ("", nil). The caller passes the + // result straight to GetNetworkByID, which does not reject an empty ID, so + // a nil error there resolves to an arbitrary network instead of failing. + t.Run("IP not associated with a network", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockAddress := cloudstack.NewMockAddressServiceIface(ctrl) + mockAddress.EXPECT().GetPublicIpAddressByID("ip-123", gomock.Any()). + Return(&cloudstack.PublicIpAddress{Id: "ip-123"}, 1, nil) + + cs := &CSCloud{ + client: &cloudstack.CloudStackClient{Address: mockAddress}, + } + + networkID, err := cs.getNetworkIDFromIPAddress("ip-123") + if err == nil { + t.Fatalf("expected an error for an IP with no associated network") + } + if networkID != "" { + t.Errorf("networkID = %q, want empty", networkID) + } + }) + + t.Run("network lookup fails", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockAddress := cloudstack.NewMockAddressServiceIface(ctrl) + mockNetwork := cloudstack.NewMockNetworkServiceIface(ctrl) + netErr := fmt.Errorf("network not found") + + gomock.InOrder( + mockAddress.EXPECT().GetPublicIpAddressByID("ip-123", gomock.Any()). + Return(&cloudstack.PublicIpAddress{Id: "ip-123", Associatednetworkid: "net-123"}, 1, nil), + mockNetwork.EXPECT().GetNetworkByID("net-123", gomock.Any()).Return(nil, 0, netErr), + ) + + cs := &CSCloud{ + client: &cloudstack.CloudStackClient{ + Address: mockAddress, + Network: mockNetwork, + }, + } + + networkID, err := cs.getNetworkIDFromIPAddress("ip-123") + if err != netErr { + t.Errorf("error = %v, want %v", err, netErr) + } + if networkID != "" { + t.Errorf("networkID = %q, want empty", networkID) + } + }) +} + +// CloudStack does not enforce unique load balancer rule names. Because +// loadBalancer.rules is keyed by name it can only manage one rule per name, so +// the rest are tracked separately and removed; before that they survived +// deletion and leaked with no service left to reference them. +// A duplicate always sits on a different public IP from the kept rule, because +// CloudStack rejects a second rule on the same IP and port. Removing only the +// load balancer rule would therefore leak the duplicate's firewall rule and +// its public IP. +func TestDuplicateLoadBalancerRules(t *testing.T) { + type mocks struct { + lb *cloudstack.MockLoadBalancerServiceIface + fw *cloudstack.MockFirewallServiceIface + addr *cloudstack.MockAddressServiceIface + } + newLB := func(t *testing.T, duplicate *cloudstack.LoadBalancerRule) (*loadBalancer, mocks) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + m := mocks{ + lb: cloudstack.NewMockLoadBalancerServiceIface(ctrl), + fw: cloudstack.NewMockFirewallServiceIface(ctrl), + addr: cloudstack.NewMockAddressServiceIface(ctrl), + } + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: m.lb, Firewall: m.fw, Address: m.addr, + }, + name: "a-lb", + ipAddrID: "ip-keep", + rules: map[string]*cloudstack.LoadBalancerRule{ + "a-lb-tcp-80": {Id: "keep", Name: "a-lb-tcp-80", Publicipid: "ip-keep"}, + }, + duplicateRules: []*cloudstack.LoadBalancerRule{duplicate}, + } + return lb, m + } + onOwnIP := &cloudstack.LoadBalancerRule{ + Id: "dup-1", Name: "a-lb-tcp-80", Publicport: "80", Protocol: "tcp", Publicipid: "ip-dup", + } + expectFirewallRules := func(m mocks, rules ...*cloudstack.FirewallRule) { + params := &cloudstack.ListFirewallRulesParams{} + m.fw.EXPECT().NewListFirewallRulesParams().Return(params) + m.fw.EXPECT().ListFirewallRules(params). + Return(&cloudstack.ListFirewallRulesResponse{FirewallRules: rules}, nil) + } + expectRuleDeleted := func(m mocks, id string, err error) { + params := &cloudstack.DeleteLoadBalancerRuleParams{} + m.lb.EXPECT().NewDeleteLoadBalancerRuleParams(id).Return(params) + m.lb.EXPECT().DeleteLoadBalancerRule(params). + Return(&cloudstack.DeleteLoadBalancerRuleResponse{}, err) + } + expectRulesOnIP := func(m mocks, count int) { + params := &cloudstack.ListLoadBalancerRulesParams{} + m.lb.EXPECT().NewListLoadBalancerRulesParams().Return(params) + m.lb.EXPECT().ListLoadBalancerRules(params). + Return(&cloudstack.ListLoadBalancerRulesResponse{Count: count}, nil) + } + + t.Run("a duplicate on its own IP takes its firewall rule and IP with it", func(t *testing.T) { + lb, m := newLB(t, onOwnIP) + expectFirewallRules(m, &cloudstack.FirewallRule{Id: "fw-dup", Protocol: "tcp", Startport: 80, Endport: 80}) + fwDelete := &cloudstack.DeleteFirewallRuleParams{} + m.fw.EXPECT().NewDeleteFirewallRuleParams("fw-dup").Return(fwDelete) + m.fw.EXPECT().DeleteFirewallRule(fwDelete).Return(&cloudstack.DeleteFirewallRuleResponse{}, nil) + expectRuleDeleted(m, "dup-1", nil) + expectRulesOnIP(m, 0) + release := &cloudstack.DisassociateIpAddressParams{} + m.addr.EXPECT().NewDisassociateIpAddressParams("ip-dup").Return(release) + m.addr.EXPECT().DisassociateIpAddress(release).Return(&cloudstack.DisassociateIpAddressResponse{}, nil) + + if err := lb.deleteDuplicateRules(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(lb.duplicateRules) != 0 { + t.Errorf("duplicateRules = %d, want 0", len(lb.duplicateRules)) + } + if kept := lb.rules["a-lb-tcp-80"]; kept == nil || kept.Id != "keep" { + t.Errorf("kept rule = %+v, want the original rule to survive", kept) + } + }) + + t.Run("the IP stays while another rule still uses it", func(t *testing.T) { + lb, m := newLB(t, onOwnIP) + expectFirewallRules(m) + expectRuleDeleted(m, "dup-1", nil) + expectRulesOnIP(m, 1) + + if err := lb.deleteDuplicateRules(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("a duplicate sharing the kept IP never releases it", func(t *testing.T) { + sharesIP := &cloudstack.LoadBalancerRule{ + Id: "dup-1", Name: "a-lb-tcp-80", Publicport: "80", Protocol: "tcp", Publicipid: "ip-keep", + } + lb, m := newLB(t, sharesIP) + expectFirewallRules(m) + expectRuleDeleted(m, "dup-1", nil) + + if err := lb.deleteDuplicateRules(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("a delete failure is surfaced", func(t *testing.T) { + lb, m := newLB(t, onOwnIP) + expectFirewallRules(m) + expectRuleDeleted(m, "dup-1", fmt.Errorf("boom")) + + if err := lb.deleteDuplicateRules(); err == nil { + t.Fatal("expected an error when deleting a duplicate fails") + } + }) + + t.Run("a duplicate with an unparseable public port is left in place without API calls", func(t *testing.T) { + lb, _ := newLB(t, &cloudstack.LoadBalancerRule{Id: "dup-1", Name: "a-lb-tcp-80", Protocol: "tcp"}) + + if err := lb.deleteDuplicateRules(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("a duplicate with an unsupported protocol is left in place without API calls", func(t *testing.T) { + lb, _ := newLB(t, &cloudstack.LoadBalancerRule{Id: "dup-1", Name: "a-lb-tcp-80", Publicport: "80", Protocol: "sctp"}) + + if err := lb.deleteDuplicateRules(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) } func TestVerifyHosts(t *testing.T) { diff --git a/cloudstack_test.go b/cloudstack_test.go index 87ed02fd..8817e043 100644 --- a/cloudstack_test.go +++ b/cloudstack_test.go @@ -195,6 +195,51 @@ func TestGetManagementServerVersion(t *testing.T) { } }) + // A version string with fewer than three components used to panic while + // being trimmed to major.minor.patch, crashing the controller at startup. + t.Run("handles short version strings without panicking", func(t *testing.T) { + for _, tc := range []struct { + version string + want semver.Version + }{ + {"4.22", semver.MustParse("4.22.0")}, + {"4", semver.MustParse("4.0.0")}, + {"24.0.0.0", semver.MustParse("24.0.0")}, + {"24.0.0.0-SNAPSHOT", semver.MustParse("24.0.0")}, + } { + t.Run(tc.version, func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockMgmt := cloudstack.NewMockManagementServiceIface(ctrl) + params := &cloudstack.ListManagementServersMetricsParams{} + resp := &cloudstack.ListManagementServersMetricsResponse{ + Count: 1, + ManagementServersMetrics: []*cloudstack.ManagementServersMetric{ + {Version: tc.version}, + }, + } + + gomock.InOrder( + mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params), + mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(resp, nil), + ) + + cs := &CSCloud{ + client: &cloudstack.CloudStackClient{Management: mockMgmt}, + } + + version, err := cs.getManagementServerVersion() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !version.Equals(tc.want) { + t.Errorf("version = %v, want %v", version, tc.want) + } + }) + } + }) + t.Run("returns error when api call fails", func(t *testing.T) { ctrl := gomock.NewController(t) t.Cleanup(ctrl.Finish) diff --git a/docs/development.md b/docs/development.md new file mode 100644 index 00000000..73f3d7d5 --- /dev/null +++ b/docs/development.md @@ -0,0 +1,424 @@ + + +# Development + +This document describes how to run a fully simulated development and test +environment for the CloudStack Kubernetes Provider: a real Kubernetes API +server, a real CloudStack management server and the CCM itself, all in +containers on your workstation. Nothing is mocked — the CCM makes genuine +CloudStack API calls and the resulting load balancer rules, public IPs and +firewall/ACL rules are real database objects you can inspect. + +The pieces are: + +| Component | What provides it | +| --- | --- | +| Kubernetes API server + kubelets | a [kind](https://kind.sigs.k8s.io/) cluster | +| CloudStack management server | the [`apache/cloudstack-simulator`](https://hub.docker.com/r/apache/cloudstack-simulator) container | +| Cloud controller manager | this repository, either in-cluster or as a host process | + +## Prerequisites + +* Docker +* [kind](https://kind.sigs.k8s.io/) v0.30 or later +* `kubectl` +* Go 1.23 or later +* [cmk](https://github.com/apache/cloudstack-cloudmonkey) (CloudMonkey), the + CloudStack CLI — the harness drives the CloudStack API through it +* `jq` and `curl` +* About 12 GB of free disk and 8 GB of RAM + +> **linux/amd64 only.** `apache/cloudstack-simulator` is not published for +> arm64. On Apple Silicon you can run it under emulation with +> `--platform linux/amd64` (expect the simulator to take three to five times +> longer to start), point the environment at a simulator running on an x86 +> host, or build an arm64 image yourself from `tools/docker/` in the +> [apache/cloudstack](https://github.com/apache/cloudstack) repository. + +## Quickstart + +```bash +make e2e-up # simulator + zone + kind cluster + CloudStack VMs + CCM +make test-e2e # phase 1: load balancer, nodes, annotations +make e2e-vpc # switch the environment to a VPC in a project +make test-e2e-vpc # phase 2: VPC / network ACL +make e2e-down # tear everything down +``` + +Phase 2 builds on phase 1, so run them in that order; both test targets stop +with a clear message if the environment they need is not up. + +`make e2e-up` takes about seven minutes once the simulator image is pulled — +roughly 90 seconds for the simulator to start, two and a half minutes to +deploy the zone, and the rest for the kind cluster, the VMs and the CCM. The +first run also has to pull a ~2 GB image. + +Once it is up, try the thing the CCM exists for: + +```bash +export KUBECONFIG=hack/e2e/_out/kubeconfig +kubectl create deployment web --image=nginx +kubectl expose deployment web --port=80 --type=LoadBalancer +kubectl get svc web -w +``` + +The service gets an `EXTERNAL-IP` from the simulator's public IP range +(`192.168.2.0/24`), and the corresponding rule shows up in CloudStack: + +```bash +cmk -c hack/e2e/_out/cmk.ini listLoadBalancerRules listall=true +``` + +The CloudStack UI is also available: run the simulator with `-p 8081:5050` +and open , logging in as `admin` / `password`. + +## How the harness talks to CloudStack + +The scripts call `cmk` directly, always in the form + +```bash +cmk -c hack/e2e/_out/cmk.ini [key=value ...] +``` + +so every CloudStack command in the harness is one you can paste into a shell. +The config file is generated by the harness rather than read from +`~/.cmk/config`, so your own cmk profiles are left alone. (cmk takes its +config path only from `-c` or `$HOME`, with no environment variable for it, +which is why the flag is repeated rather than hidden behind a wrapper.) + +The generated profile authenticates with `admin`/`password` rather than API +keys, because the harness has to talk to CloudStack *before* any keys exist — +it is what mints them. Two cmk defaults do real work here: + +* `asyncblock = true` — cmk waits for async jobs such as + `deployVirtualMachine` and returns the finished result, so nothing in the + harness polls `queryAsyncJobResult`. +* `output = json` — responses come back *without* the `response` + envelope, so a zone list is `.zone[0].id`, not + `.listzonesresponse.zone[0].id`. Worth knowing if you compare the scripts + against raw API output. + +## What the scripts do + +`hack/e2e/up.sh` chains four numbered scripts. Each is independently runnable +and safe to re-run. All tunables live in `hack/e2e/env.sh` and can be +overridden from the environment. + +### 1. `10-simulator-up.sh` — simulator and zone + +Creates a docker bridge network (`cs-ccm-e2e`, `172.30.0.0/24`) that both the +simulator and the kind nodes will join, then starts the simulator on it: + +```bash +docker network create --subnet 172.30.0.0/24 cs-ccm-e2e +docker run -d --name cloudstack-simulator --network cs-ccm-e2e \ + -p 127.0.0.1:8080:8080 apache/cloudstack-simulator:4.22.1.0 +``` + +The image exposes three ports and it matters which one you use: + +| Port | What it is | +| --- | --- | +| **8080** | the management server API (`/client/api`) — **use this one** | +| 8096 | the unauthenticated integration API, used by marvin | +| 5050 | the Vue UI development server, which proxies to 8080 | + +The upstream simulator README suggests `-p 8080:5050`, which publishes the +*UI*. For API access, publish container port 8080 directly. + +Readiness is checked in three stages rather than with a fixed sleep: jetty +answering at all, then the API accepting admin credentials, then +`listManagementServersMetrics` returning a server. The last one matters +because the CCM makes exactly that call on startup and refuses to run until it +succeeds. + +The zone is then deployed with marvin, which is preinstalled in the image: + +```bash +docker exec cloudstack-simulator python3 \ + /root/tools/marvin/marvin/deployDataCenter.py -i /root/setup/dev/advanced.cfg +``` + +This creates the `Sandbox-simulator` advanced zone with a public IP range of +`192.168.2.2`–`192.168.2.200`. + +Finally the script mints admin API keys for the CCM: + +```bash +cmk -c hack/e2e/_out/cmk.ini listUsers username=admin # -> the user id +cmk -c hack/e2e/_out/cmk.ini getUserKeys id= # -> apikey, secretkey +``` + +`listUsers` is not a substitute for `getUserKeys` — it returns the API key but +never the secret. `registerUserKeys` is used only when no key pair exists yet, +because it *rotates* the keys, which would break a simulator you are reusing. + +Keys land in `hack/e2e/_out/keys.env`. + +### 2. `20-kind-up.sh` — the Kubernetes cluster + +```bash +KIND_EXPERIMENTAL_DOCKER_NETWORK=cs-ccm-e2e kind create cluster \ + --name cs-ccm-e2e --config hack/e2e/kind-config.yaml +``` + +The cluster config does two important things: + +* `cloud-provider: external` in every node's `kubeletExtraArgs`, so nodes + register with the `node.cloudprovider.kubernetes.io/uninitialized` taint. + Removing that taint is the CCM's job, and is how you know it works. +* `kubelet-preferred-address-types: InternalIP` on the API server. Once the + CCM initializes a node it sets the node's Hostname address to the CloudStack + instance's hostname, which for the simulator is the simulated hypervisor + agent and is not resolvable. Without this setting, `kubectl logs` and + `kubectl exec` stop working after node initialization. + +The cluster is named so that node names are deterministic: +`cs-ccm-e2e-control-plane`, `cs-ccm-e2e-worker`, `cs-ccm-e2e-worker2`. Two +workers exist so tests can check that the control plane node — which kubeadm +labels `node.kubernetes.io/exclude-from-external-load-balancers` — is left out +of load balancer membership. + +The script records each node's IP on the shared docker network into +`hack/e2e/_out/node-ips`. The next step depends on it. + +> CoreDNS stays `Pending` until the CCM removes the uninitialized taint. That +> is expected; don't wait for it. + +### 3. `30-topology-isolated.sh` — matching CloudStack VMs + +**This is the part that makes or breaks the environment.** The CCM looks up +each Kubernetes node by name in CloudStack, so a VM must exist whose name +exactly matches the node name. On top of that, kind starts kubelet with +`--node-ip=`, and the CCM's node controller refuses to +initialize a node whose kubelet-reported IP is not among the addresses the +cloud provider reports for it. So the VMs must also carry the *same IP +addresses* as the kind node containers. + +The script therefore aligns the zone's guest CIDR with the docker subnet, +creates an isolated network on it, and deploys one VM per node pinned to that +node's IP: + +```bash +cmk updateZone id=$ZONE guestcidraddress=172.30.0.0/24 +cmk createNetwork name=ccm-e2e-iso networkofferingid=$OFFERING \ + gateway=172.30.0.1 netmask=255.255.255.0 zoneid=$ZONE +cmk deployVirtualMachine name=cs-ccm-e2e-worker displayname=cs-ccm-e2e-worker \ + ipaddress=172.30.0.4 networkids=$NET ... +``` + +The offering used is `DefaultIsolatedNetworkOfferingWithSourceNatService`, +which provides the **Firewall** service — so on this network the CCM manages +firewall rules. (The VPC scenario below uses an offering that provides +**NetworkACL** instead, exercising the other branch.) + +All offerings and templates are looked up by name, because their UUIDs differ +between simulator deployments. + +### 4. `40-ccm-deploy.sh` — the controller + +Generates two `cloud-config` files that differ **only in `api-url`**: + +* `hack/e2e/_out/cloud-config` — used by the in-cluster deployment, pointing + at the simulator's IP on the `cs-ccm-e2e` docker network. +* `hack/e2e/_out/cloud-config-host` — used when you run the CCM as a host + process, pointing at `http://localhost:8080/client/api`. + +The in-cluster config must use the simulator's **IP address**, not its +container name or network alias: pods have their own network namespace and +cannot reach Docker's embedded DNS resolver, and `host.docker.internal` does +not exist on Linux Docker Engine. + +Both configs set `zone` explicitly. If `zone` is empty the CCM tries to +detect it by looking up its own pod, which cannot work when running as a host +process. + +The script then loads the image into kind, applies the repository's +[`deployment.yaml`](../deployment.yaml) and patches it for testing: the local +image with `imagePullPolicy: Never`, `--leader-elect=false` (single replica, +faster startup), `--v=4` for useful logs, and higher CPU limits — the stock +manifest's `limits.cpu: 50m` throttles informer startup badly on shared CI +runners. + +Finally it waits for every node to lose the uninitialized taint. + +## Running the CCM as a host process + +For interactive development and debugging, skip step 4 and run the binary +directly against the same environment: + +```bash +make +./cloudstack-ccm \ + --cloud-provider=external-cloudstack \ + --cloud-config=hack/e2e/_out/cloud-config-host \ + --kubeconfig=hack/e2e/_out/kubeconfig \ + --leader-elect=false \ + --v=4 +``` + +If the in-cluster CCM is already running, scale it down first so the two do +not fight over the same services: + +```bash +kubectl -n kube-system scale deployment/cloud-controller-manager --replicas=0 +``` + +### Debugging + +You can use the VS Code extension +[Go](https://marketplace.visualstudio.com/items?itemName=golang.go) to debug +the CCM. Add the following to `.vscode/launch.json`: + +```json +{ + "version": "0.2.0", + "configurations": [ + { + "name": "Launch CloudStack CCM", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}/cmd/cloudstack-ccm", + "env": {}, + "args": [ + "--cloud-provider=external-cloudstack", + "--cloud-config=${workspaceFolder}/hack/e2e/_out/cloud-config-host", + "--kubeconfig=${workspaceFolder}/hack/e2e/_out/kubeconfig", + "--leader-elect=false", + "--v=4" + ], + "showLog": true, + "trace": "verbose" + }, + { + "name": "Attach to Process", + "type": "go", + "request": "attach", + "mode": "local", + "processId": 0 + } + ] +} +``` + +To debug against a real CloudStack installation instead of the simulator, +point `--cloud-config` at your own `cloud-config` and `--kubeconfig` at your +cluster's kubeconfig. + +## The VPC scenario + +`make e2e-vpc` (that is, `hack/e2e/50-topology-vpc.sh`) switches the +environment to a VPC network so the +Network ACL code path can be exercised. It creates a VPC, a **custom** ACL list, +a tier network and per-node VMs, then re-points the CCM at them and restarts it. + +Two details are worth knowing: + +* The ACL list must be a custom one. The CCM deliberately refuses to add rules + to the built-in `default_allow` and `default_deny` lists. +* Everything is created inside a **CloudStack project**. The CCM matches VM + names across the whole account, and fails with `found hosts that belong to + different networks` if the matched VMs are spread over several networks. + Because CloudStack hides project resources from non-project queries and vice + versa, putting the VPC VMs in a project makes the two scenarios mutually + invisible without needing a second cluster or a second account. + +The tier reuses the same subnet as the isolated network, so the VMs keep the +same IP addresses and node initialization continues to work after the switch. + +## Running the tests + +Unit tests need nothing but Go: + +```bash +make test +``` + +The end-to-end suite needs the environment above. It is behind the `e2e` build +tag, so it never runs as part of `make test` or `go build ./...`: + +```bash +make test-e2e # phase 1 +make test-e2e-vpc # phase 2, after `make e2e-vpc` +``` + +Configuration comes from the environment, using the same variable names as the +existing opt-in acceptance tests in `cloudstack_test.go`. The make targets set +these for you from `hack/e2e/_out/`; the table matters if you invoke `go test` +directly: + +| Variable | Meaning | +| --- | --- | +| `KUBECONFIG` | cluster under test | +| `CS_API_URL` | CloudStack API endpoint as reachable from the test process | +| `CS_API_KEY`, `CS_SECRET_KEY` | CloudStack credentials | +| `CS_PROJECT_ID` | optional; set during the VPC phase | + +When any of them is missing the tests skip rather than fail. The same suite +runs against a real CloudStack installation — just point the variables at it. + +Each test creates its own namespace and cleans up after itself. Because load +balancer provisioning is asynchronous, all assertions poll rather than +assuming immediate consistency. + +### Known limitation: provider IDs + +kind starts kubelet with `--provider-id=kind://docker//`, and +Kubernetes only allows a node's provider ID to be set once. In this +environment the CCM therefore never assigns the +`external-cloudstack://` provider ID it would set on a real +cluster. `TestNode_ProviderID` detects this, logs the value it *would* have +assigned, and reports itself as skipped, so the gap stays visible instead of +quietly passing. Everything else about node initialization — taint removal, +labels, addresses — is exercised normally. + +## Continuous integration + +[`.github/workflows/e2e-simulator.yml`](../.github/workflows/e2e-simulator.yml) +runs this environment on every pull request and every push to `main`, as a +matrix of the latest two Kubernetes versions against the latest two CloudStack +releases. The CloudStack axis is not only version coverage: CloudStack 4.22 +and later update a load balancer rule's CIDR list in place, while earlier +versions delete and recreate the rule, so both branches get tested. + +All matrix cells run in parallel and a shared build job compiles the CCM image +once, so the whole workflow takes about as long as a single run — roughly +fifteen minutes, most of it the simulator image pull and zone deployment. + +To change the versions under test, edit the `k8s` and `acs` lists in the +matrix. Both use explicit patch-level tags +([`kindest/node`](https://hub.docker.com/r/kindest/node/tags) and +[`apache/cloudstack-simulator`](https://hub.docker.com/r/apache/cloudstack-simulator/tags)), +so a run is reproducible; avoid floating tags like `latest`. + +## Troubleshooting + +| Symptom | Cause | +| --- | --- | +| `LB service provider cannot support this rule` on a VPC | The VPC virtual router accepts only a restricted set of public load balancer ports. 80 and 8080 work; an arbitrary high port such as 8081 is rejected. Pick a port the router supports when adding a VPC test. | +| CCM exits with `no management servers found` | The account cannot call `listManagementServersMetrics`. This is a root-admin API; the default `User` role does not include it. | +| Nodes keep the uninitialized taint; CCM logs `provided node ip for node "..." is not valid` | The CloudStack VM's NIC IP does not match the IP kubelet registered with. Recreate the VM with `ipaddress=` set to the kind node's docker IP. | +| Services stay ``; CCM logs `none of the hosts matched the list of VMs retrieved from CS API` | No CloudStack VM has a name matching a Kubernetes node name. | +| CCM logs `found hosts that belong to different networks` | VMs matching the node names exist on more than one network — typically leftovers from a previous scenario. | +| No ACL rules are created on a VPC network | The tier uses `default_allow` or `default_deny`. The CCM only manages rules on custom ACL lists. | +| CoreDNS stuck `Pending` | Expected until the CCM removes the uninitialized taint. If it persists, the CCM is not working — check its logs. | +| `kubectl logs`/`exec` fail after nodes initialize | The API server is preferring the Hostname address, which the CCM set to the CloudStack instance hostname. Use `kubelet-preferred-address-types: InternalIP` as the provided kind config does. | +| Simulator never becomes ready | It runs `mvn jetty:run` and fetches from Maven Central at startup. Check `docker logs cloudstack-simulator`. | diff --git a/hack/e2e/10-simulator-up.sh b/hack/e2e/10-simulator-up.sh new file mode 100755 index 00000000..46fa7fd0 --- /dev/null +++ b/hack/e2e/10-simulator-up.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Starts the CloudStack simulator, waits for it to be usable, deploys the +# advanced zone and mints admin API keys into _out/keys.env. + +set -euo pipefail +source "$(dirname "${BASH_SOURCE[0]}")/env.sh" +source "${E2E_ROOT}/lib/log.sh" +source "${E2E_ROOT}/lib/cmk.sh" + +# --- docker network shared with kind ----------------------------------------- +if ! docker network inspect "$E2E_NET" >/dev/null 2>&1; then + log "creating docker network ${E2E_NET} (${E2E_SUBNET})" + docker network create --driver bridge --subnet "$E2E_SUBNET" --gateway "$E2E_GW" "$E2E_NET" +fi + +# --- simulator container ------------------------------------------------------ +if docker inspect "$SIM_NAME" >/dev/null 2>&1 && + [[ "$(docker inspect -f '{{.Config.Image}}' "$SIM_NAME")" != "$SIM_IMAGE" ]]; then + log "simulator container ${SIM_NAME} was created from $(docker inspect -f '{{.Config.Image}}' "$SIM_NAME"), not ${SIM_IMAGE}; recreating it" + docker rm -f "$SIM_NAME" >/dev/null +fi + +# docker inspect succeeds for a stopped container too, so check the run state +# explicitly rather than "reusing" one that was never started. +if docker inspect "$SIM_NAME" >/dev/null 2>&1; then + if [[ "$(docker inspect -f '{{.State.Running}}' "$SIM_NAME")" == "true" ]]; then + log "simulator container ${SIM_NAME} already running, reusing it" + else + log "simulator container ${SIM_NAME} exists but is stopped, starting it" + docker start "$SIM_NAME" >/dev/null + fi +else + log "starting simulator ${SIM_IMAGE} as ${SIM_NAME}" + # 8080 is the management API; 5050 is only the UI dev server. + docker run -d --name "$SIM_NAME" \ + --network "$E2E_NET" --network-alias cloudstack-simulator \ + -p "127.0.0.1:${SIM_HOST_PORT}:8080" \ + "$SIM_IMAGE" +fi + +# --- staged readiness --------------------------------------------------------- +jetty_up() { + local code + code="$(curl -s -o /dev/null -w '%{http_code}' -m 5 \ + "${CS_API_URL}?command=listCapabilities&response=json")" + [[ "$code" == "401" || "$code" == "200" ]] +} + +mgmt_server_up() { + # The CCM reads this same field at startup and needs it parseable. + local version + version="$(cmk -c "$CMK_CONFIG" listManagementServersMetrics 2>/dev/null | + jq -r '.managementserver[0].version // empty')" + [[ -n "$version" ]] +} + +cmk_init + +wait_for 600 5 "jetty answering ${CS_API_URL}" jetty_up +wait_for 300 5 "CloudStack API accepting ${CS_ADMIN_USER} credentials" cmk_ready +wait_for 300 5 "management server registered" mgmt_server_up + +# --- zone --------------------------------------------------------------------- +zone_enabled() { + local state + state="$(cmk -c "$CMK_CONFIG" listZones "name=${ZONE_NAME}" | jq -r '.zone[0].allocationstate // empty')" + [[ "$state" == "Enabled" ]] +} + +host_up() { + local id + id="$(cmk -c "$CMK_CONFIG" listHosts type=Routing state=Up 2>/dev/null | + jq -r '.host[0].id // empty')" + [[ -n "$id" ]] +} + +if zone_enabled; then + log "zone ${ZONE_NAME} already deployed" +else + log "deploying zone ${ZONE_NAME} (this takes a few minutes)" + docker exec "$SIM_NAME" python3 /root/tools/marvin/marvin/deployDataCenter.py \ + -i /root/setup/dev/advanced.cfg +fi +wait_for 600 10 "zone ${ZONE_NAME} enabled" zone_enabled +wait_for 300 10 "at least one routing host up" host_up + +# --- admin API keys ----------------------------------------------------------- +admin_user_id="$(cmk -c "$CMK_CONFIG" listUsers "username=${CS_ADMIN_USER}" | jq -r '.user[0].id')" +[[ -n "$admin_user_id" && "$admin_user_id" != "null" ]] || die "could not find user ${CS_ADMIN_USER}" + +# getUserKeys first: registerUserKeys would rotate (invalidate) an existing +# pair, which is unfriendly to a long-lived local simulator. +keys="$(cmk -c "$CMK_CONFIG" getUserKeys "id=${admin_user_id}")" +api_key="$(jq -r '.userkeys.apikey // empty' <<<"$keys")" +secret_key="$(jq -r '.userkeys.secretkey // empty' <<<"$keys")" +if [[ -z "$api_key" || -z "$secret_key" ]]; then + log "no existing keys, registering new ones" + keys="$(cmk -c "$CMK_CONFIG" registerUserKeys "id=${admin_user_id}")" + api_key="$(jq -r '.userkeys.apikey' <<<"$keys")" + secret_key="$(jq -r '.userkeys.secretkey' <<<"$keys")" +fi +[[ -n "$api_key" && -n "$secret_key" ]] || die "failed to obtain admin API keys" + +cat >"${E2E_OUT}/keys.env" < docker-IP map that the CloudStack VMs must reproduce. + +set -euo pipefail +source "$(dirname "${BASH_SOURCE[0]}")/env.sh" +source "${E2E_ROOT}/lib/log.sh" + +if kind get clusters 2>/dev/null | grep -qx "$KIND_CLUSTER"; then + log "kind cluster ${KIND_CLUSTER} already exists, reusing it" +else + log "creating kind cluster ${KIND_CLUSTER} (image ${KIND_NODE_IMAGE}) on network ${E2E_NET}" + KIND_EXPERIMENTAL_DOCKER_NETWORK="$E2E_NET" kind create cluster \ + --name "$KIND_CLUSTER" \ + --config "${E2E_ROOT}/kind-config.yaml" \ + --image "$KIND_NODE_IMAGE" \ + --wait 180s +fi + +kind get kubeconfig --name "$KIND_CLUSTER" >"${E2E_OUT}/kubeconfig" +chmod 600 "${E2E_OUT}/kubeconfig" + +# The CCM only initializes a node when the CloudStack VM's NIC IP matches the +# IP kubelet registered with (kind passes --node-ip). Record each node's IP on +# the shared network so 30-topology-* can pin the VMs to them. +: >"${E2E_OUT}/node-ips" +for node in $(kind get nodes --name "$KIND_CLUSTER"); do + ip="$(docker inspect -f "{{(index .NetworkSettings.Networks \"${E2E_NET}\").IPAddress}}" "$node")" + [[ -n "$ip" ]] || die "could not determine IP of ${node} on ${E2E_NET}" + echo "${node} ${ip}" >>"${E2E_OUT}/node-ips" +done +log "node IPs:" +cat "${E2E_OUT}/node-ips" >&2 + +log "kind cluster ready; kubeconfig at ${E2E_OUT}/kubeconfig" diff --git a/hack/e2e/30-topology-isolated.sh b/hack/e2e/30-topology-isolated.sh new file mode 100755 index 00000000..3da04c69 --- /dev/null +++ b/hack/e2e/30-topology-isolated.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Creates an isolated guest network matching the kind docker subnet and +# deploys one CloudStack VM per kind node, pinned to the node's docker IP. + +set -euo pipefail +source "$(dirname "${BASH_SOURCE[0]}")/env.sh" +source "${E2E_ROOT}/lib/log.sh" +source "${E2E_ROOT}/lib/cmk.sh" + +[[ -s "${E2E_OUT}/node-ips" ]] || die "missing ${E2E_OUT}/node-ips — run 20-kind-up.sh first" +cmk_init + +zone_id="$(cmk -c "$CMK_CONFIG" listZones "name=${ZONE_NAME}" | jq -r '.zone[0].id')" +[[ -n "$zone_id" && "$zone_id" != "null" ]] || die "zone ${ZONE_NAME} not found" + +# The stock zone guest CIDR is 10.1.1.0/24; align it with the docker subnet so +# createNetwork accepts our gateway/netmask. +cmk -c "$CMK_CONFIG" updateZone "id=${zone_id}" "guestcidraddress=${E2E_SUBNET}" >/dev/null + +net_id="$(cmk -c "$CMK_CONFIG" listNetworks "keyword=${E2E_ISO_NETWORK}" listall=true | + jq -r --arg n "$E2E_ISO_NETWORK" '.network[]? | select(.name == $n) | .id')" +if [[ -z "$net_id" ]]; then + offering_id="$(cmk -c "$CMK_CONFIG" listNetworkOfferings name=DefaultIsolatedNetworkOfferingWithSourceNatService state=Enabled | + jq -r '.networkoffering[0].id')" + [[ -n "$offering_id" && "$offering_id" != "null" ]] || die "isolated network offering not found" + log "creating isolated network ${E2E_ISO_NETWORK} (${E2E_SUBNET})" + net_id="$(cmk -c "$CMK_CONFIG" createNetwork "name=${E2E_ISO_NETWORK}" "displaytext=${E2E_ISO_NETWORK}" \ + "zoneid=${zone_id}" "networkofferingid=${offering_id}" \ + "gateway=${E2E_GW}" "netmask=${E2E_NETMASK}" | + jq -r '.network.id // .id')" +fi +[[ -n "$net_id" && "$net_id" != "null" ]] || die "failed to create network ${E2E_ISO_NETWORK}" + +offering_id="$(cmk -c "$CMK_CONFIG" listServiceOfferings "name=${E2E_SERVICE_OFFERING}" | + jq -r '.serviceoffering[0].id')" +# templatefilter=executable excludes the SYSTEM (router) template, which +# cannot be used to deploy user VMs. +template_id="$(cmk -c "$CMK_CONFIG" listTemplates templatefilter=executable "zoneid=${zone_id}" hypervisor=Simulator | + jq -r '.template[]? | select(.isready == true) | .id' | head -1)" +[[ -n "$offering_id" && "$offering_id" != "null" ]] || die "service offering '${E2E_SERVICE_OFFERING}' not found" +[[ -n "$template_id" ]] || die "no ready simulator template found" + +while read -r node ip; do + existing="$(cmk -c "$CMK_CONFIG" listVirtualMachines "keyword=${node}" listall=true | + jq -r --arg n "$node" '.virtualmachine[]? | select(.name == $n) | .id')" + if [[ -n "$existing" ]]; then + log "VM ${node} already exists" + continue + fi + log "deploying VM ${node} with IP ${ip}" + cmk -c "$CMK_CONFIG" deployVirtualMachine "name=${node}" "displayname=${node}" \ + "zoneid=${zone_id}" "serviceofferingid=${offering_id}" "templateid=${template_id}" \ + "networkids=${net_id}" "ipaddress=${ip}" "startvm=true" >/dev/null +done <"${E2E_OUT}/node-ips" + +# Post-condition: every node must now have a matching VM on the right IP, or +# the CCM will never initialize that node. +while read -r node ip; do + vm_ip="$(cmk -c "$CMK_CONFIG" listVirtualMachines "keyword=${node}" listall=true | + jq -r --arg n "$node" '.virtualmachine[]? | select(.name == $n) | .nic[0].ipaddress')" + [[ "$vm_ip" == "$ip" ]] || die "VM ${node} has IP '${vm_ip}', expected ${ip}" +done <"${E2E_OUT}/node-ips" + +cat >"${E2E_OUT}/ids.env" </dev/null || die "missing ${E2E_OUT}/keys.env — run 10-simulator-up.sh first" + +# The CCM pod must reach the simulator via its IP on the shared docker +# network: pods cannot resolve docker's embedded DNS, and host.docker.internal +# does not exist on Linux. +sim_ip="$(docker inspect -f "{{(index .NetworkSettings.Networks \"${E2E_NET}\").IPAddress}}" "$SIM_NAME")" +[[ -n "$sim_ip" ]] || die "could not determine simulator IP on ${E2E_NET}" + +# PROJECT_ID is optional; 50-topology-vpc.sh re-runs this script with it set. +project_line="" +if [[ -n "${E2E_PROJECT_ID:-}" ]]; then + project_line="project-id = ${E2E_PROJECT_ID}" +fi + +# In-cluster and host-process configs differ only in api-url. +cat >"${E2E_OUT}/cloud-config" <"${E2E_OUT}/cloud-config-host" +chmod 600 "${E2E_OUT}/cloud-config" "${E2E_OUT}/cloud-config-host" + +# Rebuild unless the image was supplied from outside. Reusing whatever happens to +# carry the tag would silently test a stale binary after the checkout changes. +if [[ "$CCM_IMAGE_PREBUILT" == "true" ]]; then + docker image inspect "$CCM_IMAGE" >/dev/null 2>&1 || + die "CCM_IMAGE_PREBUILT=true but ${CCM_IMAGE} is not loaded" + log "using prebuilt ${CCM_IMAGE}" +else + log "building ${CCM_IMAGE} from ${REPO_ROOT}" + docker build -t "$CCM_IMAGE" "$REPO_ROOT" +fi +kind load docker-image "$CCM_IMAGE" --name "$KIND_CLUSTER" + +kubectl -n kube-system create secret generic cloudstack-secret \ + --from-file=cloud-config="${E2E_OUT}/cloud-config" \ + --dry-run=client -o yaml | kubectl apply -f - + +kubectl apply -f "${REPO_ROOT}/deployment.yaml" +# Adjust the stock manifest for e2e: local image, no leader election (single +# replica, faster startup), verbose logs, and enough CPU that informer startup +# is not throttled on shared runners. +kubectl -n kube-system patch deployment cloud-controller-manager --type=json -p '[ + {"op":"replace","path":"/spec/template/spec/containers/0/image","value":"'"$CCM_IMAGE"'"}, + {"op":"replace","path":"/spec/template/spec/containers/0/imagePullPolicy","value":"Never"}, + {"op":"replace","path":"/spec/template/spec/containers/0/args","value":[ + "--cloud-provider=external-cloudstack","--cloud-config=/config/cloud-config", + "--leader-elect=false","--v=4"]}, + {"op":"replace","path":"/spec/template/spec/containers/0/resources","value":{ + "requests":{"cpu":"100m","memory":"128Mi"},"limits":{"cpu":"1","memory":"512Mi"}}} +]' + +kubectl -n kube-system rollout status deployment/cloud-controller-manager --timeout=180s + +nodes_initialized() { + local taints + taints="$(kubectl get nodes -o jsonpath='{.items[*].spec.taints[?(@.key=="node.cloudprovider.kubernetes.io/uninitialized")].key}')" + [[ -z "$taints" ]] +} +# If this times out, check the CCM log for +# 'provided node ip for node ... is not valid': it means the CloudStack VM's +# NIC IP does not match the kind node's docker IP. +wait_for 300 5 "all nodes initialized by the CCM" nodes_initialized + +log "CCM deployed and all nodes initialized" +kubectl get nodes -o wide >&2 diff --git a/hack/e2e/50-topology-vpc.sh b/hack/e2e/50-topology-vpc.sh new file mode 100755 index 00000000..4dae0e74 --- /dev/null +++ b/hack/e2e/50-topology-vpc.sh @@ -0,0 +1,123 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Phase 2: creates a CloudStack project containing a VPC, a custom ACL list, a +# tier network and per-node VMs, then re-points the CCM at the project. +# +# A project is used so the VPC VMs and the phase-1 isolated-network VMs are +# mutually invisible: the CCM's verifyHosts matches VM names account-wide and +# fails when matched VMs are on different networks. With project-id set, only +# project resources are visible. The tier reuses the docker subnet, so the VMs +# get the same IPs as phase 1 and node initialization keeps working. + +set -euo pipefail +source "$(dirname "${BASH_SOURCE[0]}")/env.sh" +source "${E2E_ROOT}/lib/log.sh" +source "${E2E_ROOT}/lib/cmk.sh" + +[[ -s "${E2E_OUT}/node-ips" ]] || die "missing ${E2E_OUT}/node-ips — run 20-kind-up.sh first" +# shellcheck source=/dev/null +source "${E2E_OUT}/ids.env" 2>/dev/null || die "missing ${E2E_OUT}/ids.env — run 30-topology-isolated.sh first" +cmk_init + +# Phase-1 LoadBalancer services must be gone before the CCM switches projects, +# or their CloudStack resources leak (the project-scoped CCM can't see them). +leftover="$(kubectl get svc -A -o jsonpath='{range .items[?(@.spec.type=="LoadBalancer")]}{.metadata.namespace}/{.metadata.name}{"\n"}{end}')" +if [[ -n "$leftover" ]]; then + log "deleting leftover LoadBalancer services:" + echo "$leftover" >&2 + while IFS=/ read -r ns name; do + kubectl -n "$ns" delete svc "$name" --wait=true --timeout=120s + done <<<"$leftover" +fi + +project_id="$(cmk -c "$CMK_CONFIG" listProjects listall=true "name=${E2E_PROJECT}" | + jq -r '.project[0].id // empty')" +if [[ -z "$project_id" ]]; then + log "creating project ${E2E_PROJECT}" + project_id="$(cmk -c "$CMK_CONFIG" createProject "name=${E2E_PROJECT}" "displaytext=${E2E_PROJECT}" | + jq -r '.project.id // .id')" +fi +[[ -n "$project_id" && "$project_id" != "null" ]] || die "failed to create project" + +vpc_id="$(cmk -c "$CMK_CONFIG" listVPCs listall=true "projectid=${project_id}" "name=${E2E_VPC}" | + jq -r '.vpc[0].id // empty')" +if [[ -z "$vpc_id" ]]; then + vpc_offering_id="$(cmk -c "$CMK_CONFIG" listVPCOfferings "name=Default VPC offering" | + jq -r '.vpcoffering[0].id')" + log "creating VPC ${E2E_VPC} (${E2E_VPC_CIDR})" + vpc_id="$(cmk -c "$CMK_CONFIG" createVPC "name=${E2E_VPC}" "displaytext=${E2E_VPC}" \ + "zoneid=${E2E_ZONE_ID}" "cidr=${E2E_VPC_CIDR}" \ + "vpcofferingid=${vpc_offering_id}" "projectid=${project_id}" | + jq -r '.vpc.id // .id')" +fi +[[ -n "$vpc_id" && "$vpc_id" != "null" ]] || die "failed to create VPC" + +# A custom ACL list: the CCM refuses to manage rules on the built-in +# default_allow / default_deny lists. +acl_id="$(cmk -c "$CMK_CONFIG" listNetworkACLLists "vpcid=${vpc_id}" "name=${E2E_ACL_LIST}" | + jq -r '.networkacllist[0].id // empty')" +if [[ -z "$acl_id" ]]; then + log "creating ACL list ${E2E_ACL_LIST}" + acl_id="$(cmk -c "$CMK_CONFIG" createNetworkACLList "name=${E2E_ACL_LIST}" \ + "description=${E2E_ACL_LIST}" "vpcid=${vpc_id}" | + jq -r '.networkacllist.id // .id')" +fi +[[ -n "$acl_id" && "$acl_id" != "null" ]] || die "failed to create ACL list" + +tier_id="$(cmk -c "$CMK_CONFIG" listNetworks listall=true "projectid=${project_id}" "keyword=${E2E_TIER}" | + jq -r --arg n "$E2E_TIER" '.network[]? | select(.name == $n) | .id')" +if [[ -z "$tier_id" ]]; then + tier_offering_id="$(cmk -c "$CMK_CONFIG" listNetworkOfferings name=DefaultIsolatedNetworkOfferingForVpcNetworks state=Enabled | + jq -r '.networkoffering[0].id')" + log "creating VPC tier ${E2E_TIER} (${E2E_SUBNET})" + tier_id="$(cmk -c "$CMK_CONFIG" createNetwork "name=${E2E_TIER}" "displaytext=${E2E_TIER}" \ + "zoneid=${E2E_ZONE_ID}" "networkofferingid=${tier_offering_id}" \ + "vpcid=${vpc_id}" "aclid=${acl_id}" \ + "gateway=${E2E_GW}" "netmask=${E2E_NETMASK}" "projectid=${project_id}" | + jq -r '.network.id // .id')" +fi +[[ -n "$tier_id" && "$tier_id" != "null" ]] || die "failed to create VPC tier" + +while read -r node ip; do + existing="$(cmk -c "$CMK_CONFIG" listVirtualMachines listall=true "projectid=${project_id}" "keyword=${node}" | + jq -r --arg n "$node" '.virtualmachine[]? | select(.name == $n) | .id')" + if [[ -n "$existing" ]]; then + log "project VM ${node} already exists" + continue + fi + log "deploying project VM ${node} with IP ${ip}" + cmk -c "$CMK_CONFIG" deployVirtualMachine "name=${node}" "displayname=${node}" \ + "zoneid=${E2E_ZONE_ID}" "serviceofferingid=${E2E_SERVICE_OFFERING_ID}" \ + "templateid=${E2E_TEMPLATE_ID}" "networkids=${tier_id}" \ + "ipaddress=${ip}" "projectid=${project_id}" "startvm=true" >/dev/null +done <"${E2E_OUT}/node-ips" + +{ + echo "export E2E_PROJECT_ID='${project_id}'" + echo "export E2E_VPC_ID='${vpc_id}'" + echo "export E2E_ACL_ID='${acl_id}'" + echo "export E2E_TIER_ID='${tier_id}'" +} >>"${E2E_OUT}/ids.env" + +# Re-point the CCM at the project and restart it. +E2E_PROJECT_ID="$project_id" "${E2E_ROOT}/40-ccm-deploy.sh" +kubectl -n kube-system rollout restart deployment/cloud-controller-manager +kubectl -n kube-system rollout status deployment/cloud-controller-manager --timeout=180s + +log "VPC topology ready (project ${project_id}, tier ${tier_id}, acl ${acl_id})" diff --git a/hack/e2e/90-collect-artifacts.sh b/hack/e2e/90-collect-artifacts.sh new file mode 100755 index 00000000..94e7d90e --- /dev/null +++ b/hack/e2e/90-collect-artifacts.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Collects debugging artifacts from the simulator, the kind cluster and the +# CloudStack API into _out/artifacts. Never fails. + +set -uo pipefail +source "$(dirname "${BASH_SOURCE[0]}")/env.sh" +source "${E2E_ROOT}/lib/log.sh" +source "${E2E_ROOT}/lib/cmk.sh" + +ART="${E2E_OUT}/artifacts" +mkdir -p "$ART" + +log "collecting artifacts into ${ART}" + +docker logs "$SIM_NAME" >"${ART}/simulator.log" 2>&1 + +# kubectl logs may stop working once the CCM rewrites node addresses, so fall +# back to reading container logs on the control-plane node directly. +if ! kubectl -n kube-system logs deployment/cloud-controller-manager --tail=-1 \ + >"${ART}/ccm.log" 2>&1; then + docker exec "${KIND_CLUSTER}-control-plane" bash -c \ + 'crictl ps -a --name cloud-controller-manager -q | head -1 | xargs -r crictl logs' \ + >"${ART}/ccm.log" 2>&1 +fi + +kubectl get nodes -o yaml >"${ART}/nodes.yaml" 2>&1 +kubectl get svc -A -o yaml >"${ART}/services.yaml" 2>&1 +kubectl describe svc -A >"${ART}/svc-describe.txt" 2>&1 +kubectl get events -A --sort-by=.lastTimestamp >"${ART}/events.txt" 2>&1 +kubectl -n kube-system get pods -o wide >"${ART}/kube-system-pods.txt" 2>&1 + +# cmk_init dies when cmk is missing, which would break the "never fails" +# contract above -- this script runs from an always() CI step, where exiting +# non-zero costs the CloudStack dumps and masks the original failure. +if ! command -v cmk >/dev/null 2>&1; then + log "cmk is not installed; skipping CloudStack API dumps" +elif cmk_init && cmk_ready; then + # projectid=-1 lets an admin list across all projects, so the VPC phase shows up too. + for cmd in listLoadBalancerRules listPublicIpAddresses listFirewallRules \ + listNetworkACLs listVirtualMachines listNetworks; do + name="cs-$(echo "$cmd" | tr '[:upper:]' '[:lower:]')" + cmk -c "$CMK_CONFIG" "$cmd" listall=true | jq . >"${ART}/${name}.json" 2>&1 + cmk -c "$CMK_CONFIG" "$cmd" listall=true projectid=-1 | jq . >"${ART}/${name}-projects.json" 2>&1 + done +fi + +kind export logs "${ART}/kind" --name "$KIND_CLUSTER" >/dev/null 2>&1 + +log "artifacts collected" +exit 0 diff --git a/hack/e2e/99-down.sh b/hack/e2e/99-down.sh new file mode 100755 index 00000000..e6937773 --- /dev/null +++ b/hack/e2e/99-down.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Tears down everything the harness created. + +set -uo pipefail +source "$(dirname "${BASH_SOURCE[0]}")/env.sh" +source "${E2E_ROOT}/lib/log.sh" + +log "deleting kind cluster ${KIND_CLUSTER}" +kind delete cluster --name "$KIND_CLUSTER" 2>/dev/null + +log "removing simulator container ${SIM_NAME}" +docker rm -f "$SIM_NAME" 2>/dev/null + +log "removing docker network ${E2E_NET}" +docker network rm "$E2E_NET" 2>/dev/null + +rm -f "${E2E_OUT}/keys.env" "${E2E_OUT}/cloud-config" "${E2E_OUT}/cloud-config-host" \ + "${E2E_OUT}/kubeconfig" "${E2E_OUT}/node-ips" "${E2E_OUT}/ids.env" "${E2E_OUT}/cmk.ini" + +log "done" +exit 0 diff --git a/hack/e2e/env.sh b/hack/e2e/env.sh new file mode 100755 index 00000000..0b04957a --- /dev/null +++ b/hack/e2e/env.sh @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This file is sourced, not executed. +# shellcheck shell=bash + +# All tunables for the simulator e2e harness in one place. +# Every value can be overridden from the environment (CI does this for the +# simulator tag and the kind node image). + +E2E_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +export E2E_ROOT +export REPO_ROOT="${E2E_ROOT}/../.." +export E2E_OUT="${E2E_ROOT}/_out" + +# Docker network shared by the kind nodes and the simulator. The subnet must +# match the isolated network created in CloudStack: the CCM refuses to +# initialize a node whose kubelet-reported IP is missing from the CloudStack +# VM's NICs, so the VMs are deployed with the kind nodes' docker IPs. +export E2E_NET="${E2E_NET:-cs-ccm-e2e}" +export E2E_SUBNET="${E2E_SUBNET:-172.30.0.0/24}" +export E2E_GW="${E2E_GW:-172.30.0.1}" +export E2E_NETMASK="${E2E_NETMASK:-255.255.255.0}" + +# CloudStack simulator +export SIM_NAME="${SIM_NAME:-cloudstack-simulator}" +export SIM_TAG="${SIM_TAG:-4.22.1.0}" +export SIM_IMAGE="${SIM_IMAGE:-apache/cloudstack-simulator:${SIM_TAG}}" +export SIM_HOST_PORT="${SIM_HOST_PORT:-8080}" +export CS_API_URL="${CS_API_URL:-http://localhost:${SIM_HOST_PORT}/client/api}" +export CS_ADMIN_USER="${CS_ADMIN_USER:-admin}" +export CS_ADMIN_PASS="${CS_ADMIN_PASS:-password}" +export ZONE_NAME="${ZONE_NAME:-Sandbox-simulator}" +export E2E_REGION="${E2E_REGION:-simulator-region}" + +# kind +export KIND_CLUSTER="${KIND_CLUSTER:-cs-ccm-e2e}" +export KIND_NODE_IMAGE="${KIND_NODE_IMAGE:-kindest/node:v1.37.0}" + +# CCM image built from this checkout. Set CCM_IMAGE_PREBUILT=true when the image +# was loaded from elsewhere (CI downloads it as an artifact) so 40-ccm-deploy.sh +# uses it as-is; otherwise it is rebuilt on every run to match the working tree. +export CCM_IMAGE="${CCM_IMAGE:-apache/cloudstack-kubernetes-provider:e2e}" +export CCM_IMAGE_PREBUILT="${CCM_IMAGE_PREBUILT:-false}" + +# CloudStack names created by the harness +export E2E_ISO_NETWORK="${E2E_ISO_NETWORK:-ccm-e2e-iso}" +export E2E_PROJECT="${E2E_PROJECT:-ccm-e2e-vpc}" +export E2E_VPC="${E2E_VPC:-ccm-e2e-vpc}" +export E2E_VPC_CIDR="${E2E_VPC_CIDR:-172.30.0.0/22}" +export E2E_ACL_LIST="${E2E_ACL_LIST:-ccm-e2e-acl}" +export E2E_TIER="${E2E_TIER:-ccm-e2e-tier}" +export E2E_SERVICE_OFFERING="${E2E_SERVICE_OFFERING:-Small Instance}" + +export KUBECONFIG="${KUBECONFIG:-${E2E_OUT}/kubeconfig}" + +mkdir -p "${E2E_OUT}" diff --git a/hack/e2e/kind-config.yaml b/hack/e2e/kind-config.yaml new file mode 100644 index 00000000..5c79c241 --- /dev/null +++ b/hack/e2e/kind-config.yaml @@ -0,0 +1,62 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# kind cluster for the CloudStack simulator e2e environment. +# +# - `cloud-provider: external` makes every node register with the +# node.cloudprovider.kubernetes.io/uninitialized taint, which the CCM under +# test is responsible for removing. +# - `kubelet-preferred-address-types: InternalIP` keeps `kubectl logs`/`exec` +# working after node initialization: the CCM sets the node's Hostname +# address to the CloudStack instance hostname (the simulated hypervisor +# agent), which is not resolvable from the API server. +# - Two workers so tests can assert that the control-plane node (labeled +# node.kubernetes.io/exclude-from-external-load-balancers by kubeadm) is +# excluded from load balancer membership. +kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +name: cs-ccm-e2e +networking: + ipFamily: ipv4 + apiServerAddress: "127.0.0.1" +nodes: + - role: control-plane + kubeadmConfigPatches: + - | + kind: InitConfiguration + nodeRegistration: + kubeletExtraArgs: + cloud-provider: external + - | + kind: ClusterConfiguration + apiServer: + extraArgs: + kubelet-preferred-address-types: InternalIP + - role: worker + kubeadmConfigPatches: + - | + kind: JoinConfiguration + nodeRegistration: + kubeletExtraArgs: + cloud-provider: external + - role: worker + kubeadmConfigPatches: + - | + kind: JoinConfiguration + nodeRegistration: + kubeletExtraArgs: + cloud-provider: external diff --git a/hack/e2e/lib/cmk.sh b/hack/e2e/lib/cmk.sh new file mode 100644 index 00000000..cc73e2a1 --- /dev/null +++ b/hack/e2e/lib/cmk.sh @@ -0,0 +1,69 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This file is sourced, not executed. +# shellcheck shell=bash + +# CloudStack API access for the harness, via cmk (CloudMonkey). +# +# The scripts call cmk directly, always as: +# +# cmk -c "$CMK_CONFIG" [key=value ...] +# +# cmk resolves its config from either the -c flag or $HOME/.cmk/config and has +# no environment variable for it, so -c is passed explicitly rather than +# hijacking HOME. This keeps the developer's own ~/.cmk/config untouched, and +# means every command in the scripts is one you can paste into a shell. +# +# The generated profile authenticates with username/password rather than API +# keys, because the harness has to talk to CloudStack before any keys exist — +# it is what mints them. Two cmk defaults matter: +# +# asyncblock = true cmk waits for async jobs and returns the job result, +# so nothing here polls queryAsyncJobResult. +# output = json responses omit the response envelope, e.g. +# {"count":1,"zone":[...]}. Note that an empty result is +# zero bytes rather than {"count":0}. + +CMK_CONFIG="${E2E_OUT}/cmk.ini" + +cmk_init() { + command -v cmk >/dev/null 2>&1 || die "cmk (CloudMonkey) is not installed — see docs/development.md" + + cat >"$CMK_CONFIG" </dev/null | + jq -r '.capability.cloudstackversion // empty')" != "" ]] +} diff --git a/hack/e2e/lib/log.sh b/hack/e2e/lib/log.sh new file mode 100644 index 00000000..f4f6962c --- /dev/null +++ b/hack/e2e/lib/log.sh @@ -0,0 +1,45 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This file is sourced, not executed. +# shellcheck shell=bash + +log() { + echo "[$(date -u +%H:%M:%S)] $*" >&2 +} + +die() { + log "FATAL: $*" + exit 1 +} + +# wait_for +# Polls until it succeeds or the timeout elapses. +wait_for() { + local timeout=$1 interval=$2 desc=$3 + shift 3 + local start=$SECONDS + log "waiting up to ${timeout}s for: ${desc}" + while ((SECONDS - start < timeout)); do + if "$@" >/dev/null 2>&1; then + log "ready after $((SECONDS - start))s: ${desc}" + return 0 + fi + sleep "$interval" + done + die "timed out after ${timeout}s waiting for: ${desc}" +} diff --git a/hack/e2e/up.sh b/hack/e2e/up.sh new file mode 100755 index 00000000..9f050c90 --- /dev/null +++ b/hack/e2e/up.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# One-shot bring-up of the full simulated environment: +# simulator + zone -> kind cluster -> CloudStack VMs -> CCM. + +set -euo pipefail +here="$(dirname "${BASH_SOURCE[0]}")" + +"${here}/10-simulator-up.sh" +"${here}/20-kind-up.sh" +"${here}/30-topology-isolated.sh" +"${here}/40-ccm-deploy.sh" + +echo +echo "Environment is up. Try it:" +echo " export KUBECONFIG=${here}/_out/kubeconfig" +echo " kubectl create deployment web --image=nginx" +echo " kubectl expose deployment web --port=80 --type=LoadBalancer" +echo " kubectl get svc web -w # EXTERNAL-IP appears from 192.168.2.0/24" +echo +echo "Run the e2e suite: make test-e2e" +echo "Tear down: ${here}/99-down.sh" diff --git a/test/e2e/annotations_test.go b/test/e2e/annotations_test.go new file mode 100644 index 00000000..05548522 --- /dev/null +++ b/test/e2e/annotations_test.go @@ -0,0 +1,173 @@ +//go:build e2e + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package e2e + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/blang/semver/v4" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + annotationSourceCidrs = "service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrs" + annotationHostname = "service.beta.kubernetes.io/cloudstack-load-balancer-hostname" + annotationIPAssociated = "service.beta.kubernetes.io/cloudstack-load-balancer-ip-associated-by-controller" //nolint:gosec +) + +func TestAnnot_SourceCIDRs(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(func(s *corev1.Service) { + s.Annotations = map[string]string{ + annotationSourceCidrs: "10.0.0.0/8,192.168.100.0/24", + } + }) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + rules := f.WaitForLBRules(lbName, 1) + for _, cidr := range []string{"10.0.0.0/8", "192.168.100.0/24"} { + if !strings.Contains(rules[0].Cidrlist, cidr) { + t.Errorf("rule cidrlist = %q, want it to contain %s", rules[0].Cidrlist, cidr) + } + } + originalRuleID := rules[0].Id + + f.UpdateService(svc, func(s *corev1.Service) { + s.Annotations[annotationSourceCidrs] = "172.16.0.0/12" + }) + // Assert on the settled rule after the poll, not inside it: a failed poll + // would otherwise mask the in-place-versus-recreate check. + var settledRuleID string + f.Eventually(lbSyncTimeout, lbSyncInterval, "cidr list update to propagate", + func() (bool, error) { + current, err := f.LBRules(lbName) + if err != nil { + return false, err + } + if len(current) != 1 { + return false, fmt.Errorf("saw %d rules, want 1", len(current)) + } + if !strings.Contains(current[0].Cidrlist, "172.16.0.0/12") { + return false, fmt.Errorf("cidrlist is %q, want it to contain 172.16.0.0/12", + current[0].Cidrlist) + } + settledRuleID = current[0].Id + return true, nil + }) + + // >= 4.22 updates the rule in place; older releases delete and recreate it. + inPlace := f.Version.GTE(semver.Version{Major: 4, Minor: 22, Patch: 0}) + if inPlace && settledRuleID != originalRuleID { + t.Errorf("expected in-place cidr update on %s (rule ID changed %s -> %s)", + f.Version, originalRuleID, settledRuleID) + } + if !inPlace && settledRuleID == originalRuleID { + t.Errorf("expected rule recreation on %s (rule ID unchanged)", f.Version) + } +} + +func TestAnnot_Hostname(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(func(s *corev1.Service) { + s.Annotations = map[string]string{ + annotationHostname: "lb.example.com", + } + }) + + ingress := f.WaitForIngressIP(svc) + if ingress.Hostname != "lb.example.com" { + t.Errorf("ingress hostname = %q, want lb.example.com", ingress.Hostname) + } + if ingress.IP != "" { + t.Errorf("ingress IP = %q, want empty when hostname annotation is set", ingress.IP) + } +} + +func TestAnnot_SessionAffinity(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(func(s *corev1.Service) { + s.Spec.SessionAffinity = corev1.ServiceAffinityClientIP + }) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + rules := f.WaitForLBRules(lbName, 1) + if rules[0].Algorithm != "source" { + t.Errorf("algorithm = %q, want source for sessionAffinity ClientIP", rules[0].Algorithm) + } + + f.UpdateService(svc, func(s *corev1.Service) { + s.Spec.SessionAffinity = corev1.ServiceAffinityNone + }) + f.Eventually(lbSyncTimeout, lbSyncInterval, "algorithm to revert to roundrobin", + func() (bool, error) { + current, err := f.LBRules(lbName) + if err != nil || len(current) != 1 { + return false, err + } + return current[0].Algorithm == "roundrobin", nil + }) +} + +func TestAnnot_ExplicitLoadBalancerIP(t *testing.T) { + f := NewFramework(t) + + freeIP, err := f.FreePublicIP() + if err != nil { + t.Fatalf("finding a free public IP: %v", err) + } + + svc := f.CreateLBService(func(s *corev1.Service) { + s.Spec.LoadBalancerIP = freeIP + }) + + ingress := f.WaitForIngressIP(svc) + if ingress.IP != freeIP { + t.Fatalf("ingress IP = %q, want requested %q", ingress.IP, freeIP) + } + + // The annotation is what routes deletion through the disassociation path. + f.Eventually(lbSyncTimeout, lbSyncInterval, "ip-associated-by-controller annotation", + func() (bool, error) { + current, err := f.K8s.CoreV1().Services(svc.Namespace).Get( + context.Background(), svc.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + return current.Annotations[annotationIPAssociated] == "true", nil + }) + + f.DeleteServiceAndWait(svc) + f.Eventually(lbSyncTimeout, lbSyncInterval, "explicitly requested IP to be released", + func() (bool, error) { + ip, err := f.PublicIPByAddress(freeIP) + if err != nil || ip == nil { + return false, err + } + return ip.Allocated == "", nil + }) +} diff --git a/test/e2e/framework.go b/test/e2e/framework.go new file mode 100644 index 00000000..dbf7ecb9 --- /dev/null +++ b/test/e2e/framework.go @@ -0,0 +1,506 @@ +//go:build e2e + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Package e2e contains end-to-end tests that run against a live Kubernetes +// cluster whose cloud-controller-manager talks to a CloudStack management +// server (normally the simulator brought up by hack/e2e/up.sh). +// +// Configuration comes from the environment: +// +// KUBECONFIG kubeconfig of the cluster under test +// CS_API_URL CloudStack API endpoint (as reachable from the test process) +// CS_API_KEY CloudStack API key +// CS_SECRET_KEY CloudStack secret key +// CS_PROJECT_ID optional project scoping (set for the VPC phase) +// +// When any required variable is missing, the tests skip. +package e2e + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "os" + "strconv" + "strings" + "testing" + "time" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/blang/semver/v4" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/clientcmd" +) + +const ( + lbSyncTimeout = 3 * time.Minute + lbSyncInterval = 3 * time.Second +) + +// Framework bundles the clients and helpers shared by all e2e tests. +type Framework struct { + T *testing.T + K8s kubernetes.Interface + CS *cloudstack.CloudStackClient + Namespace string + ProjectID string + Version semver.Version +} + +// NewFramework builds clients from the environment, skipping the test when +// the environment is not configured. It creates a per-test namespace that is +// deleted on cleanup. +func NewFramework(t *testing.T) *Framework { + t.Helper() + + apiURL := os.Getenv("CS_API_URL") + apiKey := os.Getenv("CS_API_KEY") + secretKey := os.Getenv("CS_SECRET_KEY") + if apiURL == "" || apiKey == "" || secretKey == "" { + t.Skip("CS_API_URL/CS_API_KEY/CS_SECRET_KEY not set; skipping e2e test") + } + + kubeconfig := os.Getenv("KUBECONFIG") + if kubeconfig == "" { + t.Skip("KUBECONFIG not set; skipping e2e test") + } + restCfg, err := clientcmd.BuildConfigFromFlags("", kubeconfig) + if err != nil { + t.Fatalf("building kubeconfig: %v", err) + } + k8s, err := kubernetes.NewForConfig(restCfg) + if err != nil { + t.Fatalf("building kubernetes client: %v", err) + } + + verifySSL := true + if noVerify, err := strconv.ParseBool(os.Getenv("CS_SSL_NO_VERIFY")); err == nil { + verifySSL = !noVerify + } + cs := cloudstack.NewAsyncClient(apiURL, apiKey, secretKey, verifySSL) + + f := &Framework{ + T: t, + K8s: k8s, + CS: cs, + ProjectID: os.Getenv("CS_PROJECT_ID"), + } + f.Version = f.managementServerVersion() + f.Namespace = f.createNamespace() + return f +} + +func (f *Framework) managementServerVersion() semver.Version { + f.T.Helper() + resp, err := f.CS.Management.ListManagementServersMetrics( + f.CS.Management.NewListManagementServersMetricsParams()) + if err != nil { + f.T.Fatalf("listing management servers: %v", err) + } + if resp.Count == 0 { + f.T.Fatal("no management servers found") + } + raw := majorMinorPatch(resp.ManagementServersMetrics[0].Version) + v, err := semver.ParseTolerant(raw) + if err != nil { + f.T.Fatalf("parsing management server version %q: %v", raw, err) + } + return v +} + +// majorMinorPatch trims a CloudStack version such as "4.22.1.0" to its first +// three components without panicking on a shorter string. +func majorMinorPatch(version string) string { + parts := strings.Split(version, ".") + if len(parts) > 3 { + parts = parts[:3] + } + return strings.Join(parts, ".") +} + +func (f *Framework) createNamespace() string { + f.T.Helper() + buf := make([]byte, 4) + if _, err := rand.Read(buf); err != nil { + f.T.Fatalf("generating namespace suffix: %v", err) + } + name := "ccm-e2e-" + hex.EncodeToString(buf) + _, err := f.K8s.CoreV1().Namespaces().Create(context.Background(), + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}}, metav1.CreateOptions{}) + if err != nil { + f.T.Fatalf("creating namespace %s: %v", name, err) + } + f.T.Cleanup(func() { + err := f.K8s.CoreV1().Namespaces().Delete( + context.Background(), name, metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + // Deletion is asynchronous and the namespace is reaped regardless, so this only warns. + f.T.Logf("warning: deleting namespace %s: %v", name, err) + } + }) + return name +} + +// Eventually polls cond until it returns true or the timeout elapses. +func (f *Framework) Eventually(timeout, interval time.Duration, desc string, cond func() (bool, error)) { + f.T.Helper() + deadline := time.Now().Add(timeout) + var lastErr error + for time.Now().Before(deadline) { + ok, err := cond() + lastErr = err + if ok { + return + } + time.Sleep(interval) + } + if lastErr != nil { + f.T.Fatalf("timed out after %s waiting for %s; last error: %v", timeout, desc, lastErr) + } + f.T.Fatalf("timed out after %s waiting for %s; the condition was evaluated "+ + "without error but never became true", timeout, desc) +} + +// CreateLBService creates a LoadBalancer service in the test namespace and +// registers cleanup that deletes it and waits for the CloudStack rules to +// disappear, failing the test if they do not. Later tests share this +// simulator and its public IP pool, so a leaked rule has to be reported here +// rather than left to surface as an unrelated failure downstream. +func (f *Framework) CreateLBService(mutate func(*corev1.Service)) *corev1.Service { + f.T.Helper() + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e", + Namespace: f.Namespace, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Selector: map[string]string{"app": "e2e"}, + Ports: []corev1.ServicePort{ + {Name: "http", Port: 80, Protocol: corev1.ProtocolTCP}, + }, + }, + } + if mutate != nil { + mutate(svc) + } + created, err := f.K8s.CoreV1().Services(f.Namespace).Create( + context.Background(), svc, metav1.CreateOptions{}) + if err != nil { + f.T.Fatalf("creating service: %v", err) + } + f.T.Cleanup(func() { f.DeleteServiceAndWait(created) }) + return created +} + +// DeleteServiceAndWait deletes the service if it still exists, then waits for +// its CloudStack load balancer rules to be cleaned up and its public IP to be +// released. The wait runs even when the service was already gone, because the +// Kubernetes object and the CloudStack rules are torn down asynchronously. +func (f *Framework) DeleteServiceAndWait(svc *corev1.Service) { + f.T.Helper() + ingressIP := f.serviceIngressIP(svc) + + err := f.K8s.CoreV1().Services(svc.Namespace).Delete( + context.Background(), svc.Name, metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + f.T.Fatalf("deleting service %s/%s: %v", svc.Namespace, svc.Name, err) + } + + if !f.waitForLBRulesGone(defaultLoadBalancerName(svc)) { + return + } + f.settlePublicIP(ingressIP, svc) +} + +// waitForLBRulesGone reports whether the rules for lbName disappeared within +// the sync timeout. It fails with Errorf rather than Fatalf because it usually +// runs from t.Cleanup and the remaining cleanups still need to run. +func (f *Framework) waitForLBRulesGone(lbName string) bool { + f.T.Helper() + deadline := time.Now().Add(lbSyncTimeout) + var lastErr error + for time.Now().Before(deadline) { + rules, err := f.LBRules(lbName) + lastErr = err + if err == nil && len(rules) == 0 { + return true + } + time.Sleep(lbSyncInterval) + } + f.T.Errorf("load balancer rules for %s were not cleaned up within %s "+ + "(last list error: %v)", lbName, lbSyncTimeout, lastErr) + return false +} + +// settlePublicIP waits for a deleted service's public IP to leave the allocated +// state, so the next test cannot recycle an IP whose previous owner is still +// tearing down. It is a courtesy to the following test rather than an assertion +// about this one, so a timeout only logs. +func (f *Framework) settlePublicIP(ingressIP string, svc *corev1.Service) { + if ingressIP == "" { + return + } + deadline := time.Now().Add(lbSyncTimeout) + for time.Now().Before(deadline) { + ip, err := f.PublicIPByAddress(ingressIP) + if err == nil && (ip == nil || ip.Allocated == "") { + return + } + time.Sleep(lbSyncInterval) + } + f.T.Logf("warning: public IP %s was not released within %s after deleting %s/%s", + ingressIP, lbSyncTimeout, svc.Namespace, svc.Name) +} + +// serviceIngressIP returns the load balancer ingress IP currently on the +// service, or "" if the service is gone or has no ingress IP. +func (f *Framework) serviceIngressIP(svc *corev1.Service) string { + current, err := f.K8s.CoreV1().Services(svc.Namespace).Get( + context.Background(), svc.Name, metav1.GetOptions{}) + if err != nil || len(current.Status.LoadBalancer.Ingress) == 0 { + return "" + } + return current.Status.LoadBalancer.Ingress[0].IP +} + +// defaultLoadBalancerName mirrors cloudprovider.DefaultLoadBalancerName: "a" +// followed by the service UID with dashes stripped, truncated to 32 chars. +func defaultLoadBalancerName(svc *corev1.Service) string { + name := "a" + strings.ReplaceAll(string(svc.UID), "-", "") + if len(name) > 32 { + name = name[:32] + } + return name +} + +// LBRules returns the CloudStack load balancer rules whose names start with +// the given LB name. +func (f *Framework) LBRules(lbName string) ([]*cloudstack.LoadBalancerRule, error) { + p := f.CS.LoadBalancer.NewListLoadBalancerRulesParams() + p.SetKeyword(lbName) + p.SetListall(true) + if f.ProjectID != "" { + p.SetProjectid(f.ProjectID) + } + resp, err := f.CS.LoadBalancer.ListLoadBalancerRules(p) + if err != nil { + return nil, err + } + var rules []*cloudstack.LoadBalancerRule + for _, r := range resp.LoadBalancerRules { + if strings.HasPrefix(r.Name, lbName) { + rules = append(rules, r) + } + } + return rules, nil +} + +// WaitForIngressIP waits until the service has a load balancer ingress entry +// and returns it. +func (f *Framework) WaitForIngressIP(svc *corev1.Service) corev1.LoadBalancerIngress { + f.T.Helper() + var ingress corev1.LoadBalancerIngress + f.Eventually(lbSyncTimeout, lbSyncInterval, + fmt.Sprintf("service %s/%s to get an ingress address", svc.Namespace, svc.Name), + func() (bool, error) { + current, err := f.K8s.CoreV1().Services(svc.Namespace).Get( + context.Background(), svc.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + if len(current.Status.LoadBalancer.Ingress) == 0 { + return false, nil + } + ingress = current.Status.LoadBalancer.Ingress[0] + return true, nil + }) + return ingress +} + +// WaitForLBRules waits until exactly want rules exist for lbName and returns them. +func (f *Framework) WaitForLBRules(lbName string, want int) []*cloudstack.LoadBalancerRule { + f.T.Helper() + var rules []*cloudstack.LoadBalancerRule + f.Eventually(lbSyncTimeout, lbSyncInterval, + fmt.Sprintf("%d load balancer rule(s) named %s-*", want, lbName), + func() (bool, error) { + var err error + rules, err = f.LBRules(lbName) + if err != nil { + return false, err + } + if len(rules) != want { + names := make([]string, 0, len(rules)) + for _, r := range rules { + names = append(names, r.Name) + } + return false, fmt.Errorf("saw %d rule(s) %v, want %d", len(rules), names, want) + } + return true, nil + }) + return rules +} + +// FirewallRules lists the firewall rules on a public IP. +func (f *Framework) FirewallRules(publicIPID string) ([]*cloudstack.FirewallRule, error) { + p := f.CS.Firewall.NewListFirewallRulesParams() + p.SetIpaddressid(publicIPID) + p.SetListall(true) + if f.ProjectID != "" { + p.SetProjectid(f.ProjectID) + } + resp, err := f.CS.Firewall.ListFirewallRules(p) + if err != nil { + return nil, err + } + return resp.FirewallRules, nil +} + +// ACLRules lists the network ACL rules on an ACL list. +func (f *Framework) ACLRules(aclListID string) ([]*cloudstack.NetworkACL, error) { + p := f.CS.NetworkACL.NewListNetworkACLsParams() + p.SetAclid(aclListID) + p.SetListall(true) + if f.ProjectID != "" { + p.SetProjectid(f.ProjectID) + } + resp, err := f.CS.NetworkACL.ListNetworkACLs(p) + if err != nil { + return nil, err + } + return resp.NetworkACLs, nil +} + +// PublicIP fetches a public IP address record by its ID. +func (f *Framework) PublicIP(id string) (*cloudstack.PublicIpAddress, error) { + p := f.CS.Address.NewListPublicIpAddressesParams() + p.SetId(id) + p.SetListall(true) + p.SetAllocatedonly(false) + if f.ProjectID != "" { + p.SetProjectid(f.ProjectID) + } + resp, err := f.CS.Address.ListPublicIpAddresses(p) + if err != nil { + return nil, err + } + if len(resp.PublicIpAddresses) == 0 { + return nil, nil + } + return resp.PublicIpAddresses[0], nil +} + +// FreePublicIP returns an unallocated public IP address from the zone's range. +// +// Unlike PublicIP, this is deliberately not project-scoped: a free IP belongs +// to the zone's public range and has no owner yet, so filtering by project +// would exclude every candidate. +func (f *Framework) FreePublicIP() (string, error) { + p := f.CS.Address.NewListPublicIpAddressesParams() + p.SetAllocatedonly(false) + p.SetListall(true) + p.SetState("Free") + resp, err := f.CS.Address.ListPublicIpAddresses(p) + if err != nil { + return "", err + } + if len(resp.PublicIpAddresses) == 0 { + return "", fmt.Errorf("no free public IP addresses available") + } + return resp.PublicIpAddresses[0].Ipaddress, nil +} + +// PublicIPByAddress fetches a public IP address record by its address, or nil. +// +// Also not project-scoped: this is used to assert that an IP was released, and +// a released IP is no longer a project resource. Scoping it would hide exactly +// the state the assertion is looking for. +func (f *Framework) PublicIPByAddress(addr string) (*cloudstack.PublicIpAddress, error) { + p := f.CS.Address.NewListPublicIpAddressesParams() + p.SetIpaddress(addr) + p.SetAllocatedonly(false) + p.SetListall(true) + resp, err := f.CS.Address.ListPublicIpAddresses(p) + if err != nil { + return nil, err + } + if len(resp.PublicIpAddresses) == 0 { + return nil, nil + } + return resp.PublicIpAddresses[0], nil +} + +// VMByName returns the CloudStack VM with the given name, or nil. +func (f *Framework) VMByName(name string) (*cloudstack.VirtualMachine, error) { + vm, count, err := f.CS.VirtualMachine.GetVirtualMachineByName( + name, cloudstack.WithProject(f.ProjectID)) + if err != nil { + if count == 0 { + return nil, nil + } + return nil, err + } + return vm, nil +} + +// Nodes returns all nodes of the cluster under test. +func (f *Framework) Nodes() []corev1.Node { + f.T.Helper() + nodes, err := f.K8s.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{}) + if err != nil { + f.T.Fatalf("listing nodes: %v", err) + } + return nodes.Items +} + +// UpdateService applies mutate to the latest version of the service and +// updates it. A conflict means another writer (usually the CCM) won the race, +// so the service is re-read and the update retried. Any other error fails the +// test immediately: retrying it until the timeout would just bury the real +// cause under a generic "timed out" message. +func (f *Framework) UpdateService(svc *corev1.Service, mutate func(*corev1.Service)) *corev1.Service { + f.T.Helper() + var updated *corev1.Service + f.Eventually(30*time.Second, time.Second, "service update to apply without conflicting", + func() (bool, error) { + current, err := f.K8s.CoreV1().Services(svc.Namespace).Get( + context.Background(), svc.Name, metav1.GetOptions{}) + if err != nil { + f.T.Fatalf("getting service %s/%s: %v", svc.Namespace, svc.Name, err) + } + mutate(current) + updated, err = f.K8s.CoreV1().Services(svc.Namespace).Update( + context.Background(), current, metav1.UpdateOptions{}) + if apierrors.IsConflict(err) { + return false, err + } + if err != nil { + f.T.Fatalf("updating service %s/%s: %v", svc.Namespace, svc.Name, err) + } + return true, nil + }) + return updated +} diff --git a/test/e2e/loadbalancer_test.go b/test/e2e/loadbalancer_test.go new file mode 100644 index 00000000..b4918b17 --- /dev/null +++ b/test/e2e/loadbalancer_test.go @@ -0,0 +1,208 @@ +//go:build e2e + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package e2e + +import ( + "context" + "fmt" + "net" + "strconv" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestLB_CreateSingleTCPPort(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(nil) + lbName := defaultLoadBalancerName(svc) + + ingress := f.WaitForIngressIP(svc) + if ingress.IP == "" { + t.Fatalf("expected an ingress IP, got %+v", ingress) + } + if ip := net.ParseIP(ingress.IP); ip == nil { + t.Fatalf("ingress IP %q is not a valid IP", ingress.IP) + } + + rules := f.WaitForLBRules(lbName, 1) + rule := rules[0] + wantName := fmt.Sprintf("%s-tcp-80", lbName) + if rule.Name != wantName { + t.Errorf("rule name = %q, want %q", rule.Name, wantName) + } + if rule.Algorithm != "roundrobin" { + t.Errorf("rule algorithm = %q, want roundrobin", rule.Algorithm) + } + if rule.Publicport != "80" { + t.Errorf("rule public port = %q, want 80", rule.Publicport) + } + current, err := f.K8s.CoreV1().Services(svc.Namespace).Get(context.Background(), svc.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting service: %v", err) + } + nodePort := strconv.Itoa(int(current.Spec.Ports[0].NodePort)) + if rule.Privateport != nodePort { + t.Errorf("rule private port = %q, want NodePort %q", rule.Privateport, nodePort) + } + if rule.Publicip != ingress.IP { + t.Errorf("rule public IP = %q, want ingress IP %q", rule.Publicip, ingress.IP) + } + if !strings.Contains(rule.Cidrlist, "0.0.0.0/0") { + t.Errorf("rule cidrlist = %q, want it to contain 0.0.0.0/0", rule.Cidrlist) + } + + // The isolated network offering includes the Firewall service. + f.Eventually(lbSyncTimeout, lbSyncInterval, "firewall rule for port 80", + func() (bool, error) { + fwRules, err := f.FirewallRules(rule.Publicipid) + if err != nil { + return false, err + } + for _, fw := range fwRules { + if fw.Startport == 80 && fw.Endport == 80 && strings.EqualFold(fw.Protocol, "tcp") { + return true, nil + } + } + return false, nil + }) +} + +func TestLB_MultiPort(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(func(s *corev1.Service) { + s.Spec.Ports = []corev1.ServicePort{ + {Name: "http", Port: 80, Protocol: corev1.ProtocolTCP}, + {Name: "https", Port: 443, Protocol: corev1.ProtocolTCP}, + } + }) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + rules := f.WaitForLBRules(lbName, 2) + if rules[0].Publicipid != rules[1].Publicipid { + t.Errorf("expected both rules to share a public IP, got %q and %q", + rules[0].Publicipid, rules[1].Publicipid) + } + ports := map[string]bool{} + for _, r := range rules { + ports[r.Publicport] = true + } + if !ports["80"] || !ports["443"] { + t.Errorf("expected rules for ports 80 and 443, got %v", ports) + } +} + +func TestLB_NodeMembership(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(nil) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + rules := f.WaitForLBRules(lbName, 1) + + // kubeadm labels the control plane exclude-from-external-load-balancers, so only workers join the rule. + wantIDs := map[string]bool{} + for _, node := range f.Nodes() { + if _, excluded := node.Labels["node.kubernetes.io/exclude-from-external-load-balancers"]; excluded { + continue + } + vm, err := f.VMByName(node.Name) + if err != nil || vm == nil { + t.Fatalf("looking up VM for node %s: %v", node.Name, err) + } + wantIDs[vm.Id] = true + } + if len(wantIDs) == 0 { + t.Fatal("no candidate worker nodes found") + } + + f.Eventually(lbSyncTimeout, lbSyncInterval, "load balancer rule instances to match worker VMs", + func() (bool, error) { + p := f.CS.LoadBalancer.NewListLoadBalancerRuleInstancesParams(rules[0].Id) + resp, err := f.CS.LoadBalancer.ListLoadBalancerRuleInstances(p) + if err != nil { + return false, err + } + gotIDs := map[string]bool{} + for _, inst := range resp.LoadBalancerRuleInstances { + gotIDs[inst.Id] = true + } + if len(gotIDs) != len(wantIDs) { + return false, fmt.Errorf("got %d instances, want %d", len(gotIDs), len(wantIDs)) + } + for id := range wantIDs { + if !gotIDs[id] { + return false, fmt.Errorf("VM %s missing from rule instances", id) + } + } + return true, nil + }) +} + +func TestLB_PortChange(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(nil) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + f.WaitForLBRules(lbName, 1) + + f.UpdateService(svc, func(s *corev1.Service) { + s.Spec.Ports[0].Port = 8080 + }) + + f.Eventually(lbSyncTimeout, lbSyncInterval, "rule for port 8080 to replace port 80", + func() (bool, error) { + rules, err := f.LBRules(lbName) + if err != nil { + return false, err + } + return len(rules) == 1 && rules[0].Publicport == "8080", nil + }) +} + +func TestLB_Delete(t *testing.T) { + f := NewFramework(t) + svc := f.CreateLBService(nil) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + rules := f.WaitForLBRules(lbName, 1) + publicIPID := rules[0].Publicipid + + f.DeleteServiceAndWait(svc) + + if remaining, err := f.LBRules(lbName); err != nil || len(remaining) != 0 { + t.Errorf("expected no remaining rules, got %d (err %v)", len(remaining), err) + } + f.Eventually(lbSyncTimeout, lbSyncInterval, "public IP to be released", + func() (bool, error) { + ip, err := f.PublicIP(publicIPID) + if err != nil { + return false, err + } + return ip == nil || ip.Allocated == "", nil + }) +} diff --git a/test/e2e/node_test.go b/test/e2e/node_test.go new file mode 100644 index 00000000..a64fdd1e --- /dev/null +++ b/test/e2e/node_test.go @@ -0,0 +1,136 @@ +//go:build e2e + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package e2e + +import ( + "os" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" +) + +const providerIDPrefix = "external-cloudstack://" + +// TestNode_Initialized asserts the CCM removed the uninitialized taint from +// every node. +func TestNode_Initialized(t *testing.T) { + f := NewFramework(t) + for _, node := range f.Nodes() { + for _, taint := range node.Spec.Taints { + if taint.Key == "node.cloudprovider.kubernetes.io/uninitialized" { + t.Errorf("node %s still has the uninitialized taint", node.Name) + } + } + } +} + +// TestNode_ProviderID asserts every node's providerID references the matching +// CloudStack VM. +// +// kind starts kubelet with --provider-id=kind://..., and Kubernetes only lets +// the provider ID be set once, so under the kind-based harness the CCM never +// gets to assign it. Where that is the case the test verifies instead that +// the CCM would derive the right value, and reports the node as skipped so +// the limitation stays visible rather than silently reducing coverage. +func TestNode_ProviderID(t *testing.T) { + f := NewFramework(t) + checked := 0 + for _, node := range f.Nodes() { + vm, err := f.VMByName(node.Name) + if err != nil { + t.Fatalf("looking up VM for node %s: %v", node.Name, err) + } + if vm == nil { + t.Fatalf("no CloudStack VM named %s", node.Name) + } + want := providerIDPrefix + vm.Id + + if node.Spec.ProviderID != "" && !strings.HasPrefix(node.Spec.ProviderID, providerIDPrefix) { + t.Logf("node %s has a foreign provider ID %q (set by the infrastructure, "+ + "not the CCM); expected CloudStack provider ID would be %q", + node.Name, node.Spec.ProviderID, want) + continue + } + if node.Spec.ProviderID != want { + t.Errorf("node %s providerID = %q, want %q", node.Name, node.Spec.ProviderID, want) + } + checked++ + } + if checked == 0 { + t.Skip("every node has a provider ID assigned by the infrastructure; " + + "the CCM's provider ID assignment is not exercised by this environment") + } +} + +// TestNode_Labels asserts the CCM applied instance-type, zone and region +// labels from CloudStack metadata. +func TestNode_Labels(t *testing.T) { + f := NewFramework(t) + region := os.Getenv("E2E_REGION") + if region == "" { + region = "simulator-region" + } + for _, node := range f.Nodes() { + vm, err := f.VMByName(node.Name) + if err != nil || vm == nil { + t.Fatalf("looking up VM for node %s: %v", node.Name, err) + } + // Only presence is checked: labelInvalidCharsRegex rewrites the value ("Small Instance" -> "SmallInstance"). + if got := node.Labels[corev1.LabelInstanceTypeStable]; got == "" { + t.Errorf("node %s is missing label %s", node.Name, corev1.LabelInstanceTypeStable) + } + if got := node.Labels[corev1.LabelTopologyZone]; got != vm.Zonename { + t.Errorf("node %s zone label = %q, want %q", node.Name, got, vm.Zonename) + } + if got := node.Labels[corev1.LabelTopologyRegion]; got != region { + t.Errorf("node %s region label = %q, want %q", node.Name, got, region) + } + } +} + +// TestNode_InternalIP asserts each node's InternalIP equals its CloudStack +// VM's NIC address. This is the contract that makes the whole environment +// work: kubelet registers with the docker IP, and the CCM only initializes +// the node because the VM reports the same address. +func TestNode_InternalIP(t *testing.T) { + f := NewFramework(t) + for _, node := range f.Nodes() { + vm, err := f.VMByName(node.Name) + if err != nil || vm == nil { + t.Fatalf("looking up VM for node %s: %v", node.Name, err) + } + if len(vm.Nic) == 0 { + t.Fatalf("VM %s has no NICs", node.Name) + } + var internalIP string + for _, addr := range node.Status.Addresses { + if addr.Type == corev1.NodeInternalIP { + internalIP = addr.Address + } + } + if internalIP != vm.Nic[0].Ipaddress { + t.Errorf("node %s InternalIP = %q, want VM NIC IP %q", + node.Name, internalIP, vm.Nic[0].Ipaddress) + } + } +} diff --git a/test/e2e/vpc_test.go b/test/e2e/vpc_test.go new file mode 100644 index 00000000..15271eee --- /dev/null +++ b/test/e2e/vpc_test.go @@ -0,0 +1,261 @@ +//go:build e2e + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package e2e + +import ( + "context" + "os" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// vpcFramework skips unless the harness is in the VPC phase. +// +// 50-topology-vpc.sh appends E2E_ACL_ID, E2E_VPC_ID and E2E_PROJECT_ID to +// hack/e2e/_out/ids.env. Note that the project is read from CS_PROJECT_ID, not +// E2E_PROJECT_ID, because it also configures the CloudStack client in +// NewFramework; the test runner maps one to the other. To run this phase by +// hand: +// +// . hack/e2e/_out/ids.env +// CS_PROJECT_ID="$E2E_PROJECT_ID" go test -tags e2e ./test/e2e/... -run TestVPC +func vpcFramework(t *testing.T) (*Framework, string, string) { + t.Helper() + aclID := os.Getenv("E2E_ACL_ID") + vpcID := os.Getenv("E2E_VPC_ID") + if aclID == "" || vpcID == "" || os.Getenv("CS_PROJECT_ID") == "" { + t.Skip("E2E_ACL_ID/E2E_VPC_ID/CS_PROJECT_ID not set; skipping VPC phase test " + + "(CS_PROJECT_ID is set from E2E_PROJECT_ID in ids.env)") + } + return NewFramework(t), aclID, vpcID +} + +// TestVPC_LoadBalancer covers the VPC path end to end: the LB rule is +// created, the public IP is associated with the VPC, ingress traffic is +// allowed via a Network ACL rule on the custom ACL list (not a firewall +// rule), and everything is cleaned up on delete. +func TestVPC_LoadBalancer(t *testing.T) { + f, aclID, vpcID := vpcFramework(t) + + svc := f.CreateLBService(nil) + lbName := defaultLoadBalancerName(svc) + + f.WaitForIngressIP(svc) + rules := f.WaitForLBRules(lbName, 1) + rule := rules[0] + + ip, err := f.PublicIP(rule.Publicipid) + if err != nil || ip == nil { + t.Fatalf("fetching public IP %s: %v", rule.Publicipid, err) + } + if ip.Vpcid != vpcID { + t.Errorf("public IP vpcid = %q, want %q", ip.Vpcid, vpcID) + } + + f.Eventually(lbSyncTimeout, lbSyncInterval, "network ACL rule for port 80", + func() (bool, error) { + aclRules, err := f.ACLRules(aclID) + if err != nil { + return false, err + } + for _, r := range aclRules { + if r.Startport == "80" && r.Endport == "80" && + strings.EqualFold(r.Protocol, "tcp") && + strings.EqualFold(r.Action, "Allow") && + strings.EqualFold(r.Traffictype, "Ingress") { + return true, nil + } + } + return false, nil + }) + + // The tier offering has no Firewall service, so no firewall rule belongs here. + fwRules, err := f.FirewallRules(rule.Publicipid) + if err != nil { + t.Fatalf("listing firewall rules: %v", err) + } + for _, fw := range fwRules { + if fw.Startport == 80 && fw.Endport == 80 { + t.Errorf("unexpected firewall rule on VPC public IP: %+v", fw) + } + } + + f.DeleteServiceAndWait(svc) + f.Eventually(lbSyncTimeout, lbSyncInterval, "network ACL rule to be removed", + func() (bool, error) { + aclRules, err := f.ACLRules(aclID) + if err != nil { + return false, err + } + for _, r := range aclRules { + if r.Startport == "80" && r.Endport == "80" && strings.EqualFold(r.Protocol, "tcp") { + return false, nil + } + } + return true, nil + }) +} + +// TestVPC_NodesReinitialized asserts the CCM re-initialized the nodes against +// the project VMs after the phase switch. +func TestVPC_NodesReinitialized(t *testing.T) { + f, _, _ := vpcFramework(t) + for _, node := range f.Nodes() { + vm, err := f.VMByName(node.Name) + if err != nil { + t.Fatalf("looking up project VM for node %s: %v", node.Name, err) + } + if vm == nil { + t.Errorf("no project VM named %s visible with CS_PROJECT_ID", node.Name) + } + } + for _, node := range f.Nodes() { + for _, taint := range node.Spec.Taints { + if taint.Key == "node.cloudprovider.kubernetes.io/uninitialized" { + t.Errorf("node %s still has the uninitialized taint", node.Name) + } + } + } +} + +// countACLRules returns how many ingress ACL rules on the list target a port. +func countACLRules(f *Framework, aclID, port string) (int, error) { + rules, err := f.ACLRules(aclID) + if err != nil { + return 0, err + } + n := 0 + for _, r := range rules { + if r.Startport == port && r.Endport == port && strings.EqualFold(r.Protocol, "tcp") { + n++ + } + } + return n, nil +} + +// TestVPC_ACLRuleNotDuplicatedOnResync is a regression test for a +// project-scoping bug in updateNetworkACL: it listed the existing ACL rules +// without the project, so with project-id set it never saw the rule it had +// just created and appended another one on every reconcile. +func TestVPC_ACLRuleNotDuplicatedOnResync(t *testing.T) { + f, aclID, _ := vpcFramework(t) + + // An ACL rule belongs to the tier, not the service, so sharing port 80 with + // TestVPC_LoadBalancer would blur the count. + const port = "8080" + svc := f.CreateLBService(func(s *corev1.Service) { + s.Spec.Ports = []corev1.ServicePort{ + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + } + }) + lbName := defaultLoadBalancerName(svc) + f.WaitForIngressIP(svc) + + f.Eventually(lbSyncTimeout, lbSyncInterval, "the ACL rule for port "+port, + func() (bool, error) { + n, err := countACLRules(f, aclID, port) + return n >= 1, err + }) + + forceReconcile(f, svc, lbName) + + n, err := countACLRules(f, aclID, port) + if err != nil { + t.Fatalf("counting ACL rules: %v", err) + } + if n != 1 { + t.Errorf("ACL rules for port %s = %d, want exactly 1; the reconcile duplicated the rule", port, n) + } +} + +// forceReconcile makes the service controller run EnsureLoadBalancer again by +// flipping sessionAffinity, then waits for the resulting algorithm change so the +// caller observes a reconcile that has demonstrably completed. +func forceReconcile(f *Framework, svc *corev1.Service, lbName string) { + f.T.Helper() + f.UpdateService(svc, func(s *corev1.Service) { + s.Spec.SessionAffinity = corev1.ServiceAffinityClientIP + }) + f.Eventually(lbSyncTimeout, lbSyncInterval, "the reconcile to apply the new algorithm", + func() (bool, error) { + rules, err := f.LBRules(lbName) + if err != nil || len(rules) != 1 { + return false, err + } + return rules[0].Algorithm == "source", nil + }) +} + +// TestVPC_ExplicitLoadBalancerIPReleased is a regression test for a +// project-scoping bug in EnsureLoadBalancerDeleted: the disassociation check +// looked the public IP up without the project, so with project-id set the +// lookup failed, the controller decided not to disassociate, and the IP leaked +// on every deletion. +// +// It only reproduces with spec.loadBalancerIP set: an auto-allocated IP is +// released unconditionally and never reaches that check. +func TestVPC_ExplicitLoadBalancerIPReleased(t *testing.T) { + f, _, _ := vpcFramework(t) + + freeIP, err := f.FreePublicIP() + if err != nil { + t.Fatalf("finding a free public IP: %v", err) + } + + // The VPC virtual router rejects most public ports with "LB service provider + // cannot support this rule"; 80 is one it accepts. + svc := f.CreateLBService(func(s *corev1.Service) { + s.Spec.LoadBalancerIP = freeIP + s.Spec.Ports = []corev1.ServicePort{ + {Name: "http", Port: 80, Protocol: corev1.ProtocolTCP}, + } + }) + + if ingress := f.WaitForIngressIP(svc); ingress.IP != freeIP { + t.Fatalf("ingress IP = %q, want requested %q", ingress.IP, freeIP) + } + + // The annotation is what routes deletion through the disassociation path under test. + f.Eventually(lbSyncTimeout, lbSyncInterval, "the ip-associated-by-controller annotation", + func() (bool, error) { + current, err := f.K8s.CoreV1().Services(svc.Namespace).Get( + context.Background(), svc.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + return current.Annotations[annotationIPAssociated] == "true", nil + }) + + f.DeleteServiceAndWait(svc) + + f.Eventually(lbSyncTimeout, lbSyncInterval, "the explicitly requested IP to be released", + func() (bool, error) { + ip, err := f.PublicIPByAddress(freeIP) + if err != nil || ip == nil { + return false, err + } + return ip.Allocated == "", nil + }) +}