diff --git a/.github/workflows/go-coverage.yml b/.github/workflows/go-coverage.yml index 09abf4a6..0d195b57 100644 --- a/.github/workflows/go-coverage.yml +++ b/.github/workflows/go-coverage.yml @@ -9,12 +9,12 @@ on: jobs: build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.21' + go-version: '1.22' - name: Run go test with coverage run: COVER_PROFILE=coverage.txt make test - name: Codecov upload diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 27cb3c07..e528f495 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -6,12 +6,12 @@ on: jobs: lint: name: Lint - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: Install Go 1.x uses: actions/setup-go@v5 with: - go-version: '1.21' + go-version: '1.22' - name: Check out code uses: actions/checkout@v4 @@ -21,7 +21,7 @@ jobs: build: name: Test & Build - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: Install Go 1.x uses: actions/setup-go@v5 @@ -30,6 +30,8 @@ jobs: - name: Check out code uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Cache uses: actions/cache@v4 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ed6af4cf..a5d3ecee 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,11 +16,13 @@ env: jobs: push: name: Push images - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: Check out code uses: actions/checkout@v4 + with: + fetch-depth: 0 # This step is run when the branch is main and no tag is set - name: Sets env vars for main run: | @@ -64,7 +66,7 @@ jobs: release: name: Release - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 # Run only if previous job has succeeded needs: [push] diff --git a/.golangci.yml b/.golangci.yml index e4adf9c8..1e747287 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -32,11 +32,12 @@ linters-settings: linters: enable: - - gosec - - goheader - - revive - gocyclo + - goheader + - gosec - misspell + - revive + - staticcheck run: issues-exit-code: 1 @@ -51,3 +52,6 @@ issues: - path: _test\.go linters: - gosec + - text: "SA1019: .+LBRuleID is deprecated" + linters: + - staticcheck \ No newline at end of file diff --git a/Makefile b/Makefile index 43f58495..6b1cc0d4 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,9 @@ GH_REPO ?= kubernetes-sigs/cluster-api-provider-cloudstack # Helper function to get dependency version from go.mod get_go_version = $(shell go list -m $1 | awk '{print $$2}') +# Set build time variables including version details +LDFLAGS := $(shell source ./hack/version.sh; version::ldflags) + # Binaries KUSTOMIZE_VER := v4.5.7 KUSTOMIZE_BIN := kustomize @@ -92,15 +95,10 @@ MOCKGEN_VER := v1.6.0 MOCKGEN := $(abspath $(TOOLS_BIN_DIR)/$(MOCKGEN_BIN)-$(MOCKGEN_VER)) MOCKGEN_PKG := github.com/golang/mock/mockgen -STATIC_CHECK_BIN := staticcheck -STATIC_CHECK_VER := v0.4.7 -STATIC_CHECK := $(abspath $(TOOLS_BIN_DIR)/staticcheck) -STATIC_CHECK_PKG := honnef.co/go/tools/cmd/staticcheck - KUBECTL := $(TOOLS_BIN_DIR)/kubectl # Release -STAGING_REGISTRY := gcr.io/k8s-staging-capi-cloudstack +STAGING_REGISTRY := ghcr.io/leaseweb STAGING_BUCKET ?= artifacts.k8s-staging-capi-cloudstack.appspot.com BUCKET ?= $(STAGING_BUCKET) PROD_REGISTRY ?= registry.k8s.io/capi-cloudstack @@ -112,7 +110,7 @@ RELEASE_ALIAS_TAG ?= $(PULL_BASE_REF) # Image URL to use all building/pushing image targets REGISTRY ?= $(STAGING_REGISTRY) IMAGE_NAME ?= capi-cloudstack-controller -TAG ?= dev +TAG ?= develop CONTROLLER_IMG ?= $(REGISTRY)/$(IMAGE_NAME) IMG ?= $(CONTROLLER_IMG):$(TAG) IMG_LOCAL ?= localhost:5000/$(IMAGE_NAME):$(TAG) @@ -143,7 +141,7 @@ all: build ## -------------------------------------- .PHONY: binaries -binaries: $(CONTROLLER_GEN) $(CONVERSION_GEN) $(GOLANGCI_LINT) $(STATIC_CHECK) $(GINKGO) $(MOCKGEN) $(KUSTOMIZE) $(SETUP_ENVTEST) managers # Builds and installs all binaries +binaries: $(CONTROLLER_GEN) $(CONVERSION_GEN) $(GOLANGCI_LINT) $(GINKGO) $(MOCKGEN) $(KUSTOMIZE) $(SETUP_ENVTEST) managers # Builds and installs all binaries .PHONY: managers managers: @@ -167,17 +165,10 @@ vet: ## Run go vet on the whole project. go vet ./... .PHONY: lint -lint: $(GOLANGCI_LINT) $(STATIC_CHECK) generate-mocks ## Run linting for the project. +lint: $(GOLANGCI_LINT) generate-mocks ## Run linting for the project. $(MAKE) fmt $(MAKE) vet $(GOLANGCI_LINT) run -v --timeout 360s ./... - $(STATIC_CHECK) ./... - @ # The below string of commands checks that ginkgo isn't present in the controllers. - @(grep ginkgo ${REPO_ROOT}/controllers/cloudstack*_controller.go | grep -v import && \ - echo "Remove ginkgo from controllers. This is probably an artifact of testing." \ - "See the hack/testing_ginkgo_recover_statements.sh file") && exit 1 || \ - echo "Gingko statements not found in controllers... (passed)" - ##@ Generate ## -------------------------------------- @@ -186,11 +177,11 @@ lint: $(GOLANGCI_LINT) $(STATIC_CHECK) generate-mocks ## Run linting for the pro .PHONY: modules modules: ## Runs go mod to ensure proper vendoring. - go mod tidy -compat=1.21 - cd $(TOOLS_DIR); go mod tidy -compat=1.21 + go mod tidy -compat=1.22 + cd $(TOOLS_DIR); go mod tidy -compat=1.22 .PHONY: generate-all -generate-all: generate-mocks generate-deepcopy generate-manifests +generate-all: generate-mocks generate-conversion generate-deepcopy generate-manifests .PHONY: generate-mocks generate-mocks: $(MOCKGEN) generate-deepcopy pkg/mocks/mock_client.go $(shell find ./pkg/mocks -type f -name "mock*.go") ## Generate mocks needed for testing. Primarily mocks of the cloud package. @@ -235,13 +226,13 @@ MANAGER_BIN_INPUTS=$(shell find ./controllers ./api ./pkg -name "*mock*" -prune .PHONY: build build: binaries generate-deepcopy lint generate-manifests release-manifests ## Build manager binary. $(BIN_DIR)/manager: $(MANAGER_BIN_INPUTS) - go build -o $(BIN_DIR)/manager main.go + go build -ldflags "${LDFLAGS}" -o $(BIN_DIR)/manager main.go .PHONY: build-for-docker build-for-docker: $(BIN_DIR)/manager-linux-amd64 ## Build manager binary for docker image building. $(BIN_DIR)/manager-linux-amd64: $(MANAGER_BIN_INPUTS) CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \ - go build -a -ldflags "${ldflags} -extldflags '-static'" \ + go build -a -ldflags "${LDFLAGS} -extldflags '-static'" \ -o $(BIN_DIR)/manager-linux-amd64 main.go .PHONY: run @@ -333,11 +324,8 @@ setup-envtest: $(SETUP_ENVTEST) ## Set up envtest (download kubebuilder assets) .PHONY: test test: ## Run tests. -test: generate-deepcopy-test generate-manifest-test generate-mocks lint setup-envtest $(GINKGO) - @./hack/testing_ginkgo_recover_statements.sh --add # Add ginkgo.GinkgoRecover() statements to controllers. - @# The following is a slightly funky way to make sure the ginkgo statements are removed regardless the test results. - KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" $(GINKGO) --label-filter="!integ" --cover -coverprofile cover.out --covermode=atomic -v ./api/... ./controllers/... ./pkg/...; EXIT_STATUS=$$?;\ - ./hack/testing_ginkgo_recover_statements.sh --remove; exit $$EXIT_STATUS +test: generate-deepcopy-test generate-manifest-test generate-mocks setup-envtest $(GINKGO) + KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" $(GINKGO) --label-filter="!integ" --cover -coverprofile cover.out --covermode=atomic -v ./api/... ./controllers/... ./pkg/... .PHONY: test-pkg test-pkg: $(GINKGO) ## Run pkg tests. @@ -452,9 +440,6 @@ $(GOLANGCI_LINT_BIN): $(GOLANGCI_LINT) ## Build a local copy of golangci-lint. .PHONY: $(MOCKGEN_BIN) $(MOCKGEN_BIN): $(MOCKGEN) ## Build a local copy of mockgen. -.PHONY: $(STATIC_CHECK_BIN) -$(STATIC_CHECK_BIN): $(STATIC_CHECK) ## Build a local copy of staticcheck. - $(CONTROLLER_GEN): # Build controller-gen from tools folder. GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(CONTROLLER_GEN_PKG) $(CONTROLLER_GEN_BIN) $(CONTROLLER_GEN_VER) @@ -481,6 +466,3 @@ $(GOLANGCI_LINT): # Build golangci-lint from tools folder. $(MOCKGEN): # Build mockgen from tools folder. GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(MOCKGEN_PKG) $(MOCKGEN_BIN) $(MOCKGEN_VER) - -$(STATIC_CHECK): # Build golangci-lint from tools folder. - GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(STATIC_CHECK_PKG) $(STATIC_CHECK_BIN) $(STATIC_CHECK_VER) diff --git a/api/v1beta1/cloudstackisolatednetwork_conversion.go b/api/v1beta1/cloudstackisolatednetwork_conversion.go index 4fe3e577..37a64b49 100644 --- a/api/v1beta1/cloudstackisolatednetwork_conversion.go +++ b/api/v1beta1/cloudstackisolatednetwork_conversion.go @@ -19,6 +19,7 @@ package v1beta1 import ( machineryconversion "k8s.io/apimachinery/pkg/conversion" "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" + infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" utilconversion "sigs.k8s.io/cluster-api/util/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" ) @@ -53,3 +54,19 @@ func (dst *CloudStackIsolatedNetwork) ConvertFrom(srcRaw conversion.Hub) error { func Convert_v1beta3_CloudStackIsolatedNetworkSpec_To_v1beta1_CloudStackIsolatedNetworkSpec(in *v1beta3.CloudStackIsolatedNetworkSpec, out *CloudStackIsolatedNetworkSpec, s machineryconversion.Scope) error { // nolint return autoConvert_v1beta3_CloudStackIsolatedNetworkSpec_To_v1beta1_CloudStackIsolatedNetworkSpec(in, out, s) } + +func Convert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(in *CloudStackIsolatedNetworkStatus, out *v1beta3.CloudStackIsolatedNetworkStatus, s machineryconversion.Scope) error { + out.PublicIPID = in.PublicIPID + out.LBRuleID = in.LBRuleID + out.APIServerLoadBalancer = &infrav1.LoadBalancer{} + out.LoadBalancerRuleIDs = []string{in.LBRuleID} + out.Ready = in.Ready + return nil +} + +func Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus(in *v1beta3.CloudStackIsolatedNetworkStatus, out *CloudStackIsolatedNetworkStatus, s machineryconversion.Scope) error { + out.PublicIPID = in.PublicIPID + out.LBRuleID = in.LBRuleID + out.Ready = in.Ready + return nil +} diff --git a/api/v1beta1/cloudstackmachinetemplate_conversion.go b/api/v1beta1/cloudstackmachinetemplate_conversion.go index 93c3cd33..bd672f87 100644 --- a/api/v1beta1/cloudstackmachinetemplate_conversion.go +++ b/api/v1beta1/cloudstackmachinetemplate_conversion.go @@ -17,13 +17,12 @@ limitations under the License. package v1beta1 import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" machineryconversion "k8s.io/apimachinery/pkg/conversion" "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" utilconversion "sigs.k8s.io/cluster-api/util/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) func (src *CloudStackMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { // nolint @@ -43,6 +42,9 @@ func (src *CloudStackMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { / if restored.Spec.Template.Spec.UncompressedUserData != nil { dst.Spec.Template.Spec.UncompressedUserData = restored.Spec.Template.Spec.UncompressedUserData } + + dst.Spec.Template.ObjectMeta = restored.Spec.Template.ObjectMeta + return nil } diff --git a/api/v1beta1/conversion.go b/api/v1beta1/conversion.go index 59b8839f..08b48fcb 100644 --- a/api/v1beta1/conversion.go +++ b/api/v1beta1/conversion.go @@ -18,8 +18,7 @@ package v1beta1 import ( "context" - "fmt" - + "errors" corev1 "k8s.io/api/core/v1" machineryconversion "k8s.io/apimachinery/pkg/conversion" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" @@ -51,7 +50,7 @@ func Convert_v1beta1_CloudStackCluster_To_v1beta3_CloudStackCluster(in *CloudSta //nolint:golint,revive,stylecheck func Convert_v1beta3_CloudStackCluster_To_v1beta1_CloudStackCluster(in *infrav1.CloudStackCluster, out *CloudStackCluster, _ machineryconversion.Scope) error { if len(in.Spec.FailureDomains) < 1 { - return fmt.Errorf("infrav1 to v1beta1 conversion not supported when < 1 failure domain is provided. Input CloudStackCluster spec %s", in.Spec) + return errors.New("v1beta3 to v1beta1 conversion not supported when < 1 failure domain is provided") } out.ObjectMeta = in.ObjectMeta out.Spec = CloudStackClusterSpec{ diff --git a/api/v1beta1/conversion_test.go b/api/v1beta1/conversion_test.go index 32ffcd37..4da5fbe6 100644 --- a/api/v1beta1/conversion_test.go +++ b/api/v1beta1/conversion_test.go @@ -21,6 +21,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" v1beta1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta1" "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -103,6 +104,11 @@ var _ = Describe("Conversion", func() { Host: "endpoint1", Port: 443, }, + APIServerLoadBalancer: &v1beta3.APIServerLoadBalancer{ + Enabled: pointer.Bool(true), + AdditionalPorts: []int{}, + AllowedCIDRs: []string{}, + }, }, Status: v1beta3.CloudStackClusterStatus{}, } diff --git a/api/v1beta1/v1beta1_suite_test.go b/api/v1beta1/v1beta1_suite_test.go new file mode 100644 index 00000000..4c49498e --- /dev/null +++ b/api/v1beta1/v1beta1_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed 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 v1beta1_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestV1beta1(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "V1beta1 Suite") +} diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 851a2286..a053a29a 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -98,16 +98,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*CloudStackIsolatedNetworkStatus)(nil), (*v1beta3.CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(a.(*CloudStackIsolatedNetworkStatus), b.(*v1beta3.CloudStackIsolatedNetworkStatus), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta3.CloudStackIsolatedNetworkStatus)(nil), (*CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus(a.(*v1beta3.CloudStackIsolatedNetworkStatus), b.(*CloudStackIsolatedNetworkStatus), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*CloudStackMachine)(nil), (*v1beta3.CloudStackMachine)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_CloudStackMachine_To_v1beta3_CloudStackMachine(a.(*CloudStackMachine), b.(*v1beta3.CloudStackMachine), scope) }); err != nil { @@ -248,6 +238,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*CloudStackIsolatedNetworkStatus)(nil), (*v1beta3.CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(a.(*CloudStackIsolatedNetworkStatus), b.(*v1beta3.CloudStackIsolatedNetworkStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*CloudStackMachineSpec)(nil), (*v1beta3.CloudStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_CloudStackMachineSpec_To_v1beta3_CloudStackMachineSpec(a.(*CloudStackMachineSpec), b.(*v1beta3.CloudStackMachineSpec), scope) }); err != nil { @@ -278,6 +273,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta3.CloudStackIsolatedNetworkStatus)(nil), (*CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus(a.(*v1beta3.CloudStackIsolatedNetworkStatus), b.(*CloudStackIsolatedNetworkStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta3.CloudStackMachineSpec)(nil), (*CloudStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta3_CloudStackMachineSpec_To_v1beta1_CloudStackMachineSpec(a.(*v1beta3.CloudStackMachineSpec), b.(*CloudStackMachineSpec), scope) }); err != nil { @@ -506,6 +506,7 @@ func autoConvert_v1beta3_CloudStackIsolatedNetworkSpec_To_v1beta1_CloudStackIsol out.ID = in.ID out.ControlPlaneEndpoint = in.ControlPlaneEndpoint // WARNING: in.FailureDomainName requires manual conversion: does not exist in peer-type + // WARNING: in.CIDR requires manual conversion: does not exist in peer-type // WARNING: in.Domain requires manual conversion: does not exist in peer-type return nil } @@ -517,23 +518,16 @@ func autoConvert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIs return nil } -// Convert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus is an autogenerated conversion function. -func Convert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(in *CloudStackIsolatedNetworkStatus, out *v1beta3.CloudStackIsolatedNetworkStatus, s conversion.Scope) error { - return autoConvert_v1beta1_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(in, out, s) -} - func autoConvert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus(in *v1beta3.CloudStackIsolatedNetworkStatus, out *CloudStackIsolatedNetworkStatus, s conversion.Scope) error { + // WARNING: in.PublicIPAddress requires manual conversion: does not exist in peer-type out.PublicIPID = in.PublicIPID out.LBRuleID = in.LBRuleID + // WARNING: in.LoadBalancerRuleIDs requires manual conversion: does not exist in peer-type + // WARNING: in.APIServerLoadBalancer requires manual conversion: does not exist in peer-type out.Ready = in.Ready return nil } -// Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus is an autogenerated conversion function. -func Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus(in *v1beta3.CloudStackIsolatedNetworkStatus, out *CloudStackIsolatedNetworkStatus, s conversion.Scope) error { - return autoConvert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta1_CloudStackIsolatedNetworkStatus(in, out, s) -} - func autoConvert_v1beta1_CloudStackMachine_To_v1beta3_CloudStackMachine(in *CloudStackMachine, out *v1beta3.CloudStackMachine, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1beta1_CloudStackMachineSpec_To_v1beta3_CloudStackMachineSpec(&in.Spec, &out.Spec, s); err != nil { @@ -979,6 +973,7 @@ func autoConvert_v1beta3_Network_To_v1beta1_Network(in *v1beta3.Network, out *Ne out.ID = in.ID out.Type = in.Type out.Name = in.Name + // WARNING: in.CIDR requires manual conversion: does not exist in peer-type // WARNING: in.Domain requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta2/cloudstackcluster_conversion.go b/api/v1beta2/cloudstackcluster_conversion.go index 31fd3c20..ed47000e 100644 --- a/api/v1beta2/cloudstackcluster_conversion.go +++ b/api/v1beta2/cloudstackcluster_conversion.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta2 import ( + machineryconversion "k8s.io/apimachinery/pkg/conversion" + "k8s.io/utils/pointer" "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" "sigs.k8s.io/controller-runtime/pkg/conversion" ) @@ -30,3 +32,25 @@ func (dst *CloudStackCluster) ConvertFrom(srcRaw conversion.Hub) error { // noli src := srcRaw.(*v1beta3.CloudStackCluster) return Convert_v1beta3_CloudStackCluster_To_v1beta2_CloudStackCluster(src, dst, nil) } + +func Convert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(in *v1beta3.CloudStackClusterSpec, out *CloudStackClusterSpec, s machineryconversion.Scope) error { // nolint + err := autoConvert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(in, out, s) + if err != nil { + return err + } + + return nil +} + +func Convert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec(in *CloudStackClusterSpec, out *v1beta3.CloudStackClusterSpec, s machineryconversion.Scope) error { // nolint + err := autoConvert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec(in, out, s) + if err != nil { + return err + } + + out.APIServerLoadBalancer = &v1beta3.APIServerLoadBalancer{ + Enabled: pointer.Bool(true), + } + + return nil +} diff --git a/api/v1beta2/cloudstackisolatednetwork_conversion.go b/api/v1beta2/cloudstackisolatednetwork_conversion.go index 44e824ff..51cccc8d 100644 --- a/api/v1beta2/cloudstackisolatednetwork_conversion.go +++ b/api/v1beta2/cloudstackisolatednetwork_conversion.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta2 import ( + machineryconversion "k8s.io/apimachinery/pkg/conversion" "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" "sigs.k8s.io/controller-runtime/pkg/conversion" ) @@ -30,3 +31,18 @@ func (dst *CloudStackIsolatedNetwork) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*v1beta3.CloudStackIsolatedNetwork) return Convert_v1beta3_CloudStackIsolatedNetwork_To_v1beta2_CloudStackIsolatedNetwork(src, dst, nil) } + +func Convert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(in *CloudStackIsolatedNetworkStatus, out *v1beta3.CloudStackIsolatedNetworkStatus, s machineryconversion.Scope) error { + out.PublicIPID = in.PublicIPID + out.LBRuleID = in.LBRuleID + out.LoadBalancerRuleIDs = []string{in.LBRuleID} + out.Ready = in.Ready + return nil +} + +func Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus(in *v1beta3.CloudStackIsolatedNetworkStatus, out *CloudStackIsolatedNetworkStatus, s machineryconversion.Scope) error { + out.PublicIPID = in.PublicIPID + out.LBRuleID = in.LBRuleID + out.Ready = in.Ready + return nil +} diff --git a/api/v1beta2/cloudstackmachinetemplate_conversion.go b/api/v1beta2/cloudstackmachinetemplate_conversion.go index 85964140..7621936b 100644 --- a/api/v1beta2/cloudstackmachinetemplate_conversion.go +++ b/api/v1beta2/cloudstackmachinetemplate_conversion.go @@ -42,6 +42,9 @@ func (src *CloudStackMachineTemplate) ConvertTo(dstRaw conversion.Hub) error { / if restored.Spec.Template.Spec.UncompressedUserData != nil { dst.Spec.Template.Spec.UncompressedUserData = restored.Spec.Template.Spec.UncompressedUserData } + + dst.Spec.Template.ObjectMeta = restored.Spec.Template.ObjectMeta + return nil } diff --git a/api/v1beta2/zz_generated.conversion.go b/api/v1beta2/zz_generated.conversion.go index 41dd8333..de8d49d4 100644 --- a/api/v1beta2/zz_generated.conversion.go +++ b/api/v1beta2/zz_generated.conversion.go @@ -98,16 +98,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*CloudStackClusterSpec)(nil), (*v1beta3.CloudStackClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec(a.(*CloudStackClusterSpec), b.(*v1beta3.CloudStackClusterSpec), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta3.CloudStackClusterSpec)(nil), (*CloudStackClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(a.(*v1beta3.CloudStackClusterSpec), b.(*CloudStackClusterSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*CloudStackClusterStatus)(nil), (*v1beta3.CloudStackClusterStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta2_CloudStackClusterStatus_To_v1beta3_CloudStackClusterStatus(a.(*CloudStackClusterStatus), b.(*v1beta3.CloudStackClusterStatus), scope) }); err != nil { @@ -183,16 +173,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*CloudStackIsolatedNetworkStatus)(nil), (*v1beta3.CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(a.(*CloudStackIsolatedNetworkStatus), b.(*v1beta3.CloudStackIsolatedNetworkStatus), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta3.CloudStackIsolatedNetworkStatus)(nil), (*CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus(a.(*v1beta3.CloudStackIsolatedNetworkStatus), b.(*CloudStackIsolatedNetworkStatus), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*CloudStackMachine)(nil), (*v1beta3.CloudStackMachine)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta2_CloudStackMachine_To_v1beta3_CloudStackMachine(a.(*CloudStackMachine), b.(*v1beta3.CloudStackMachine), scope) }); err != nil { @@ -338,6 +318,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*CloudStackClusterSpec)(nil), (*v1beta3.CloudStackClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec(a.(*CloudStackClusterSpec), b.(*v1beta3.CloudStackClusterSpec), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*CloudStackIsolatedNetworkStatus)(nil), (*v1beta3.CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(a.(*CloudStackIsolatedNetworkStatus), b.(*v1beta3.CloudStackIsolatedNetworkStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*CloudStackMachineSpec)(nil), (*v1beta3.CloudStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta2_CloudStackMachineSpec_To_v1beta3_CloudStackMachineSpec(a.(*CloudStackMachineSpec), b.(*v1beta3.CloudStackMachineSpec), scope) }); err != nil { @@ -348,11 +338,21 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta3.CloudStackClusterSpec)(nil), (*CloudStackClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(a.(*v1beta3.CloudStackClusterSpec), b.(*CloudStackClusterSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta3.CloudStackIsolatedNetworkSpec)(nil), (*CloudStackIsolatedNetworkSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta3_CloudStackIsolatedNetworkSpec_To_v1beta2_CloudStackIsolatedNetworkSpec(a.(*v1beta3.CloudStackIsolatedNetworkSpec), b.(*CloudStackIsolatedNetworkSpec), scope) }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta3.CloudStackIsolatedNetworkStatus)(nil), (*CloudStackIsolatedNetworkStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus(a.(*v1beta3.CloudStackIsolatedNetworkStatus), b.(*CloudStackIsolatedNetworkStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta3.CloudStackMachineSpec)(nil), (*CloudStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta3_CloudStackMachineSpec_To_v1beta2_CloudStackMachineSpec(a.(*v1beta3.CloudStackMachineSpec), b.(*CloudStackMachineSpec), scope) }); err != nil { @@ -561,11 +561,6 @@ func autoConvert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec( return nil } -// Convert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec is an autogenerated conversion function. -func Convert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec(in *CloudStackClusterSpec, out *v1beta3.CloudStackClusterSpec, s conversion.Scope) error { - return autoConvert_v1beta2_CloudStackClusterSpec_To_v1beta3_CloudStackClusterSpec(in, out, s) -} - func autoConvert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(in *v1beta3.CloudStackClusterSpec, out *CloudStackClusterSpec, s conversion.Scope) error { if in.FailureDomains != nil { in, out := &in.FailureDomains, &out.FailureDomains @@ -579,14 +574,10 @@ func autoConvert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec( out.FailureDomains = nil } out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + // WARNING: in.APIServerLoadBalancer requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec is an autogenerated conversion function. -func Convert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(in *v1beta3.CloudStackClusterSpec, out *CloudStackClusterSpec, s conversion.Scope) error { - return autoConvert_v1beta3_CloudStackClusterSpec_To_v1beta2_CloudStackClusterSpec(in, out, s) -} - func autoConvert_v1beta2_CloudStackClusterStatus_To_v1beta3_CloudStackClusterStatus(in *CloudStackClusterStatus, out *v1beta3.CloudStackClusterStatus, s conversion.Scope) error { out.FailureDomains = *(*v1beta1.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) out.Ready = in.Ready @@ -827,6 +818,7 @@ func autoConvert_v1beta3_CloudStackIsolatedNetworkSpec_To_v1beta2_CloudStackIsol out.ID = in.ID out.ControlPlaneEndpoint = in.ControlPlaneEndpoint out.FailureDomainName = in.FailureDomainName + // WARNING: in.CIDR requires manual conversion: does not exist in peer-type // WARNING: in.Domain requires manual conversion: does not exist in peer-type return nil } @@ -838,23 +830,16 @@ func autoConvert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIs return nil } -// Convert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus is an autogenerated conversion function. -func Convert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(in *CloudStackIsolatedNetworkStatus, out *v1beta3.CloudStackIsolatedNetworkStatus, s conversion.Scope) error { - return autoConvert_v1beta2_CloudStackIsolatedNetworkStatus_To_v1beta3_CloudStackIsolatedNetworkStatus(in, out, s) -} - func autoConvert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus(in *v1beta3.CloudStackIsolatedNetworkStatus, out *CloudStackIsolatedNetworkStatus, s conversion.Scope) error { + // WARNING: in.PublicIPAddress requires manual conversion: does not exist in peer-type out.PublicIPID = in.PublicIPID out.LBRuleID = in.LBRuleID + // WARNING: in.LoadBalancerRuleIDs requires manual conversion: does not exist in peer-type + // WARNING: in.APIServerLoadBalancer requires manual conversion: does not exist in peer-type out.Ready = in.Ready return nil } -// Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus is an autogenerated conversion function. -func Convert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus(in *v1beta3.CloudStackIsolatedNetworkStatus, out *CloudStackIsolatedNetworkStatus, s conversion.Scope) error { - return autoConvert_v1beta3_CloudStackIsolatedNetworkStatus_To_v1beta2_CloudStackIsolatedNetworkStatus(in, out, s) -} - func autoConvert_v1beta2_CloudStackMachine_To_v1beta3_CloudStackMachine(in *CloudStackMachine, out *v1beta3.CloudStackMachine, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1beta2_CloudStackMachineSpec_To_v1beta3_CloudStackMachineSpec(&in.Spec, &out.Spec, s); err != nil { @@ -1305,6 +1290,7 @@ func autoConvert_v1beta3_Network_To_v1beta2_Network(in *v1beta3.Network, out *Ne out.ID = in.ID out.Type = in.Type out.Name = in.Name + // WARNING: in.CIDR requires manual conversion: does not exist in peer-type // WARNING: in.Domain requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta3/cloudstackcluster_types.go b/api/v1beta3/cloudstackcluster_types.go index 309caefd..837dd0eb 100644 --- a/api/v1beta3/cloudstackcluster_types.go +++ b/api/v1beta3/cloudstackcluster_types.go @@ -34,6 +34,11 @@ type CloudStackClusterSpec struct { // The kubernetes control plane endpoint. ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"` + + // APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. + // If not specified, no load balancer will be created for the API server. + //+optional + APIServerLoadBalancer *APIServerLoadBalancer `json:"apiServerLoadBalancer,omitempty"` } // The status of the CloudStackCluster object. diff --git a/api/v1beta3/cloudstackcluster_webhook.go b/api/v1beta3/cloudstackcluster_webhook.go index 3158394e..0ec6af1b 100644 --- a/api/v1beta3/cloudstackcluster_webhook.go +++ b/api/v1beta3/cloudstackcluster_webhook.go @@ -18,6 +18,7 @@ package v1beta3 import ( "fmt" + "net" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -79,6 +80,12 @@ func (r *CloudStackCluster) ValidateCreate() (admission.Warnings, error) { field.NewPath("spec", "failureDomains", "ACSEndpoint"), "Name and Namespace are required")) } + if fdSpec.Zone.Network.CIDR != "" { + if _, errMsg := ValidateCIDR(fdSpec.Zone.Network.CIDR); errMsg != nil { + errorList = append(errorList, field.Invalid( + field.NewPath("spec", "failureDomains", "Zone", "Network"), fdSpec.Zone.Network.CIDR, fmt.Sprintf("must be valid CIDR: %s", errMsg.Error()))) + } + } if fdSpec.Zone.Network.Domain != "" { for _, errMsg := range validation.IsDNS1123Subdomain(fdSpec.Zone.Network.Domain) { errorList = append(errorList, field.Invalid( @@ -127,6 +134,13 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) (admission.Warnin return nil, webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList) } +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type +func (r *CloudStackCluster) ValidateDelete() (admission.Warnings, error) { + cloudstackclusterlog.V(1).Info("entered validate delete webhook", "api resource name", r.Name) + // No deletion validations. Deletion webhook not enabled. + return nil, nil +} + // ValidateFailureDomainUpdates verifies that at least one failure domain has not been deleted, and // failure domains that are held over have not been modified. func ValidateFailureDomainUpdates(oldFDs, newFDs []CloudStackFailureDomainSpec) *field.Error { @@ -165,9 +179,11 @@ func FailureDomainsEqual(fd1, fd2 CloudStackFailureDomainSpec) bool { fd1.Zone.Network.Domain == fd2.Zone.Network.Domain } -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type -func (r *CloudStackCluster) ValidateDelete() (admission.Warnings, error) { - cloudstackclusterlog.V(1).Info("entered validate delete webhook", "api resource name", r.Name) - // No deletion validations. Deletion webhook not enabled. - return nil, nil +// ValidateCIDR validates whether a CIDR matches the conventions expected by net.ParseCIDR +func ValidateCIDR(cidr string) (*net.IPNet, error) { + _, net, err := net.ParseCIDR(cidr) + if err != nil { + return nil, err + } + return net, nil } diff --git a/api/v1beta3/cloudstackcluster_webhook_test.go b/api/v1beta3/cloudstackcluster_webhook_test.go index 0ec212b5..bfcdb5e0 100644 --- a/api/v1beta3/cloudstackcluster_webhook_test.go +++ b/api/v1beta3/cloudstackcluster_webhook_test.go @@ -30,6 +30,7 @@ import ( var _ = Describe("CloudStackCluster webhooks", func() { var ctx context.Context forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s" + invalidRegex := "admission webhook.*denied the request.*Invalid value\\: \".*\"\\: %s" requiredRegex := "admission webhook.*denied the request.*Required value\\: %s" BeforeEach(func() { // Reset test vars to initial state. @@ -44,6 +45,12 @@ var _ = Describe("CloudStackCluster webhooks", func() { Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(Succeed()) }) + It("Should reject a CloudStackCluster with invalid failure domain name", func() { + dummies.CSCluster.Spec.FailureDomains[0].Name = "bogus_failuredomain" + Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(invalidRegex, + "a lowercase RFC 1123 subdomain must consist of"))) + }) + It("Should reject a CloudStackCluster with missing Zones.Network attribute", func() { dummies.CSCluster.Spec.FailureDomains = []infrav1.CloudStackFailureDomainSpec{{}} dummies.CSCluster.Spec.FailureDomains[0].Zone.Name = "ZoneWNoNetwork" @@ -56,6 +63,18 @@ var _ = Describe("CloudStackCluster webhooks", func() { Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(requiredRegex, "each Zone requires a Network specification"))) }) + + It("Should reject a CloudStackCluster with invalid network domain", func() { + dummies.CSCluster.Spec.FailureDomains[0].Zone.Network.Domain = "bogus_domain.example.com" + Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(invalidRegex, + "a lowercase RFC 1123 subdomain must consist of"))) + }) + + It("Should reject a CloudStackCluster with invalid network CIDR", func() { + dummies.CSCluster.Spec.FailureDomains[0].Zone.Network.CIDR = "111.222.333.444/55" + Ω(k8sClient.Create(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(invalidRegex, + "must be valid CIDR: invalid CIDR address: 111.222.333.444/55"))) + }) }) Context("When updating a CloudStackCluster", func() { diff --git a/api/v1beta3/cloudstackfailuredomain_types.go b/api/v1beta3/cloudstackfailuredomain_types.go index be3f5e48..893f0f88 100644 --- a/api/v1beta3/cloudstackfailuredomain_types.go +++ b/api/v1beta3/cloudstackfailuredomain_types.go @@ -54,6 +54,10 @@ type Network struct { // Cloudstack Network Name the cluster is built in. Name string `json:"name"` + // CIDR is the IP address range of the network. + //+optional + CIDR string `json:"cidr,omitempty"` + // Domain is the DNS domain name used for all instances in the network. //+optional Domain string `json:"domain,omitempty"` diff --git a/api/v1beta3/cloudstackisolatednetwork_types.go b/api/v1beta3/cloudstackisolatednetwork_types.go index 1e5db27d..7c2f45ce 100644 --- a/api/v1beta3/cloudstackisolatednetwork_types.go +++ b/api/v1beta3/cloudstackisolatednetwork_types.go @@ -40,6 +40,10 @@ type CloudStackIsolatedNetworkSpec struct { // FailureDomainName -- the FailureDomain the network is placed in. FailureDomainName string `json:"failureDomainName"` + // CIDR is the IP range of the isolated network. + //+optional + CIDR string `json:"cidr,omitempty"` + // Domain is the DNS domain name used for all instances in the isolated network. //+optional Domain string `json:"domain,omitempty"` @@ -47,12 +51,23 @@ type CloudStackIsolatedNetworkSpec struct { // CloudStackIsolatedNetworkStatus defines the observed state of CloudStackIsolatedNetwork type CloudStackIsolatedNetworkStatus struct { - // The CS public IP ID to use for the k8s endpoint. + // The outgoing IP of the isolated network. + PublicIPAddress string `json:"publicIPAddress,omitempty"` + + // The CS public IP ID of the outgoing IP of the isolated network. PublicIPID string `json:"publicIPID,omitempty"` - // The ID of the lb rule used to assign VMs to the lb. + // Deprecated: The ID of the lb rule used to assign VMs to the lb. + // No longer used, see LoadBalancerRuleIDs. Will be removed in next API version. LBRuleID string `json:"loadBalancerRuleID,omitempty"` + // The IDs of the lb rule used to assign VMs to the lb. + LoadBalancerRuleIDs []string `json:"loadBalancerRuleIDs,omitempty"` + + // APIServerLoadBalancer describes the api server load balancer if one exists + //+optional + APIServerLoadBalancer *LoadBalancer `json:"apiServerLoadBalancer,omitempty"` + // Ready indicates the readiness of this provider resource. //+optional Ready bool `json:"ready"` diff --git a/api/v1beta3/types.go b/api/v1beta3/types.go new file mode 100644 index 00000000..d2f73a35 --- /dev/null +++ b/api/v1beta3/types.go @@ -0,0 +1,57 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed 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 v1beta3 + +// LoadBalancer represents basic information about the associated OpenStack LoadBalancer. +type LoadBalancer struct { + IPAddress string `json:"ipAddress"` + IPAddressID string `json:"ipAddressID"` + //+optional + AllowedCIDRs []string `json:"allowedCIDRs,omitempty"` +} + +type APIServerLoadBalancer struct { + // Enabled defines whether a load balancer should be created. This value + // defaults to true if an APIServerLoadBalancer is given. + // + // There is no reason to set this to false. To disable creation of the + // API server loadbalancer, omit the APIServerLoadBalancer field in the + // cluster spec instead. + // + //+kubebuilder:validation:Required + //+kubebuilder:default:=true + Enabled *bool `json:"enabled"` + + // AdditionalPorts adds additional tcp ports to the load balancer. + //+optional + //+listType=set + AdditionalPorts []int `json:"additionalPorts,omitempty"` + + // AllowedCIDRs restrict access to all API-Server listeners to the given address CIDRs. + //+optional + //+listType=set + AllowedCIDRs []string `json:"allowedCIDRs,omitempty"` +} + +func (s *APIServerLoadBalancer) IsZero() bool { + return s == nil || ((s.Enabled == nil || !*s.Enabled) && len(s.AdditionalPorts) == 0 && len(s.AllowedCIDRs) == 0) +} + +func (s *APIServerLoadBalancer) IsEnabled() bool { + // The CRD default value for Enabled is true, so if the field is nil, it should be considered as true. + return s != nil && (s.Enabled == nil || *s.Enabled) +} diff --git a/api/v1beta3/zz_generated.deepcopy.go b/api/v1beta3/zz_generated.deepcopy.go index 66a2e486..92f5538c 100644 --- a/api/v1beta3/zz_generated.deepcopy.go +++ b/api/v1beta3/zz_generated.deepcopy.go @@ -26,6 +26,36 @@ import ( "sigs.k8s.io/cluster-api/api/v1beta1" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *APIServerLoadBalancer) DeepCopyInto(out *APIServerLoadBalancer) { + *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + if in.AdditionalPorts != nil { + in, out := &in.AdditionalPorts, &out.AdditionalPorts + *out = make([]int, len(*in)) + copy(*out, *in) + } + if in.AllowedCIDRs != nil { + in, out := &in.AllowedCIDRs, &out.AllowedCIDRs + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIServerLoadBalancer. +func (in *APIServerLoadBalancer) DeepCopy() *APIServerLoadBalancer { + if in == nil { + return nil + } + out := new(APIServerLoadBalancer) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CloudStackAffinityGroup) DeepCopyInto(out *CloudStackAffinityGroup) { *out = *in @@ -183,6 +213,11 @@ func (in *CloudStackClusterSpec) DeepCopyInto(out *CloudStackClusterSpec) { copy(*out, *in) } out.ControlPlaneEndpoint = in.ControlPlaneEndpoint + if in.APIServerLoadBalancer != nil { + in, out := &in.APIServerLoadBalancer, &out.APIServerLoadBalancer + *out = new(APIServerLoadBalancer) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloudStackClusterSpec. @@ -314,7 +349,7 @@ func (in *CloudStackIsolatedNetwork) DeepCopyInto(out *CloudStackIsolatedNetwork out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloudStackIsolatedNetwork. @@ -386,6 +421,16 @@ func (in *CloudStackIsolatedNetworkSpec) DeepCopy() *CloudStackIsolatedNetworkSp // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CloudStackIsolatedNetworkStatus) DeepCopyInto(out *CloudStackIsolatedNetworkStatus) { *out = *in + if in.LoadBalancerRuleIDs != nil { + in, out := &in.LoadBalancerRuleIDs, &out.LoadBalancerRuleIDs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.APIServerLoadBalancer != nil { + in, out := &in.APIServerLoadBalancer, &out.APIServerLoadBalancer + *out = new(LoadBalancer) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloudStackIsolatedNetworkStatus. @@ -769,6 +814,26 @@ func (in *CloudStackZoneSpec) DeepCopy() *CloudStackZoneSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LoadBalancer) DeepCopyInto(out *LoadBalancer) { + *out = *in + if in.AllowedCIDRs != nil { + in, out := &in.AllowedCIDRs, &out.AllowedCIDRs + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancer. +func (in *LoadBalancer) DeepCopy() *LoadBalancer { + if in == nil { + return nil + } + out := new(LoadBalancer) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Network) DeepCopyInto(out *Network) { *out = *in diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml index 26ef8b09..ed1de549 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml @@ -353,6 +353,39 @@ spec: spec: description: CloudStackClusterSpec defines the desired state of CloudStackCluster. properties: + apiServerLoadBalancer: + description: |- + APIServerLoadBalancer configures the optional LoadBalancer for the APIServer. + If not specified, no load balancer will be created for the API server. + properties: + additionalPorts: + description: AdditionalPorts adds additional tcp ports to the + load balancer. + items: + type: integer + type: array + x-kubernetes-list-type: set + allowedCIDRs: + description: AllowedCIDRs restrict access to all API-Server listeners + to the given address CIDRs. + items: + type: string + type: array + x-kubernetes-list-type: set + enabled: + default: true + description: |- + Enabled defines whether a load balancer should be created. This value + defaults to true if an APIServerLoadBalancer is given. + + + There is no reason to set this to false. To disable creation of the + API server loadbalancer, omit the APIServerLoadBalancer field in the + cluster spec instead. + type: boolean + required: + - enabled + type: object controlPlaneEndpoint: description: The kubernetes control plane endpoint. properties: @@ -406,6 +439,9 @@ spec: network: description: The network within the Zone to use. properties: + cidr: + description: CIDR is the IP address range of the network. + type: string domain: description: Domain is the DNS domain name used for all instances in the network. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml index 5d59fa92..e794b516 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackfailuredomains.yaml @@ -175,6 +175,9 @@ spec: network: description: The network within the Zone to use. properties: + cidr: + description: CIDR is the IP address range of the network. + type: string domain: description: Domain is the DNS domain name used for all instances in the network. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml index 7fa68b52..b7032f5b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackisolatednetworks.yaml @@ -188,6 +188,9 @@ spec: description: CloudStackIsolatedNetworkSpec defines the desired state of CloudStackIsolatedNetwork properties: + cidr: + description: CIDR is the IP range of the isolated network. + type: string controlPlaneEndpoint: description: The kubernetes control plane endpoint. properties: @@ -224,11 +227,38 @@ spec: description: CloudStackIsolatedNetworkStatus defines the observed state of CloudStackIsolatedNetwork properties: + apiServerLoadBalancer: + description: APIServerLoadBalancer describes the api server load balancer + if one exists + properties: + allowedCIDRs: + items: + type: string + type: array + ipAddress: + type: string + ipAddressID: + type: string + required: + - ipAddress + - ipAddressID + type: object loadBalancerRuleID: - description: The ID of the lb rule used to assign VMs to the lb. + description: |- + Deprecated: The ID of the lb rule used to assign VMs to the lb. + No longer used, see LoadBalancerRuleIDs. Will be removed in next API version. + type: string + loadBalancerRuleIDs: + description: The IDs of the lb rule used to assign VMs to the lb. + items: + type: string + type: array + publicIPAddress: + description: The outgoing IP of the isolated network. type: string publicIPID: - description: The CS public IP ID to use for the k8s endpoint. + description: The CS public IP ID of the outgoing IP of the isolated + network. type: string ready: description: Ready indicates the readiness of this provider resource. diff --git a/controllers/cloudstackfailuredomain_controller.go b/controllers/cloudstackfailuredomain_controller.go index fb5f7384..519abfc4 100644 --- a/controllers/cloudstackfailuredomain_controller.go +++ b/controllers/cloudstackfailuredomain_controller.go @@ -106,11 +106,10 @@ func (r *CloudStackFailureDomainReconciliationRunner) Reconcile() (retRes ctrl.R // CloudStackIsolatedNetwork to manage the many intricacies and wait until CloudStackIsolatedNetwork is ready. if r.ReconciliationSubject.Spec.Zone.Network.ID == "" || r.ReconciliationSubject.Spec.Zone.Network.Type == infrav1.NetworkTypeIsolated { - netName := r.ReconciliationSubject.Spec.Zone.Network.Name if res, err := r.GenerateIsolatedNetwork( - netName, r.ReconciliationSubject.Spec.Name, r.ReconciliationSubject.Spec.Zone.Network.Domain)(); r.ShouldReturn(res, err) { + r.ReconciliationSubject.Spec.Zone.Network, r.ReconciliationSubject.Spec.Name)(); r.ShouldReturn(res, err) { return res, err - } else if res, err := r.GetObjectByName(r.IsoNetMetaName(netName), r.IsoNet)(); r.ShouldReturn(res, err) { + } else if res, err := r.GetObjectByName(r.IsoNetMetaName(r.ReconciliationSubject.Spec.Zone.Network.Name), r.IsoNet)(); r.ShouldReturn(res, err) { return res, err } if r.IsoNet.Name == "" { diff --git a/controllers/cloudstackisolatednetwork_controller.go b/controllers/cloudstackisolatednetwork_controller.go index b9943245..eb97f3bd 100644 --- a/controllers/cloudstackisolatednetwork_controller.go +++ b/controllers/cloudstackisolatednetwork_controller.go @@ -18,6 +18,12 @@ package controllers import ( "context" + "reflect" + "sigs.k8s.io/cluster-api/util/annotations" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" "strings" "sigs.k8s.io/cluster-api/util/patch" @@ -40,7 +46,7 @@ type CloudStackIsoNetReconciler struct { csCtrlrUtils.ReconcilerBase } -// CloudStackZoneReconciliationRunner is a ReconciliationRunner with extensions specific to CloudStack isolated network reconciliation. +// CloudStackIsoNetReconciliationRunner is a ReconciliationRunner with extensions specific to CloudStack isolated network reconciliation. type CloudStackIsoNetReconciliationRunner struct { *csCtrlrUtils.ReconciliationRunner FailureDomain *infrav1.CloudStackFailureDomain @@ -67,18 +73,19 @@ func (reconciler *CloudStackIsoNetReconciler) Reconcile(ctx context.Context, req return r.RunBaseReconciliationStages() } -func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (retRes ctrl.Result, retErr error) { +func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (ctrl.Result, error) { controllerutil.AddFinalizer(r.ReconciliationSubject, infrav1.IsolatedNetworkFinalizer) // Setup isolated network, endpoint, egress, and load balancing. // Set endpoint of CloudStackCluster if it is not currently set. (uses patcher to do so) csClusterPatcher, err := patch.NewHelper(r.CSCluster, r.K8sClient) if err != nil { - return r.ReturnWrappedError(retErr, "setting up CloudStackCluster patcher") + return ctrl.Result{}, errors.Wrap(err, "setting up CloudStackCluster patcher") } if r.FailureDomain.Spec.Zone.ID == "" { return r.RequeueWithMessage("Zone ID not resolved yet.") } + if err := r.CSUser.GetOrCreateIsolatedNetwork(r.FailureDomain, r.ReconciliationSubject, r.CSCluster); err != nil { return ctrl.Result{}, err } @@ -86,11 +93,39 @@ func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (retRes ctrl.Result, if err := r.CSUser.AddClusterTag(cloud.ResourceTypeNetwork, r.ReconciliationSubject.Spec.ID, r.CSCluster); err != nil { return ctrl.Result{}, errors.Wrapf(err, "tagging network with id %s", r.ReconciliationSubject.Spec.ID) } + + // Assign IP and configure API server load balancer, if enabled and this cluster is not externally managed. + if !annotations.IsExternallyManaged(r.CSCluster) { + pubIP, err := r.CSUser.AssociatePublicIPAddress(r.FailureDomain, r.ReconciliationSubject, r.CSCluster.Spec.ControlPlaneEndpoint.Host) + if err != nil { + return ctrl.Result{}, errors.Wrap(err, "associating public IP address") + } + r.ReconciliationSubject.Spec.ControlPlaneEndpoint.Host = pubIP.Ipaddress + r.CSCluster.Spec.ControlPlaneEndpoint.Host = pubIP.Ipaddress + r.ReconciliationSubject.Status.PublicIPID = pubIP.Id + r.ReconciliationSubject.Status.PublicIPAddress = pubIP.Ipaddress + + if r.ReconciliationSubject.Status.APIServerLoadBalancer == nil { + r.ReconciliationSubject.Status.APIServerLoadBalancer = &infrav1.LoadBalancer{} + } + r.ReconciliationSubject.Status.APIServerLoadBalancer.IPAddressID = pubIP.Id + r.ReconciliationSubject.Status.APIServerLoadBalancer.IPAddress = pubIP.Ipaddress + if err := r.CSUser.AddClusterTag(cloud.ResourceTypeIPAddress, pubIP.Id, r.CSCluster); err != nil { + return ctrl.Result{}, errors.Wrapf(err, + "adding cluster tag to public IP address with ID %s", pubIP.Id) + } + + if err := r.CSUser.ReconcileLoadBalancer(r.FailureDomain, r.ReconciliationSubject, r.CSCluster); err != nil { + return ctrl.Result{}, errors.Wrap(err, "reconciling load balancer") + } + } + if err := csClusterPatcher.Patch(r.RequestCtx, r.CSCluster); err != nil { - return r.ReturnWrappedError(err, "patching endpoint update to CloudStackCluster") + return ctrl.Result{}, errors.Wrap(err, "patching endpoint update to CloudStackCluster") } r.ReconciliationSubject.Status.Ready = true + return ctrl.Result{}, nil } @@ -107,8 +142,42 @@ func (r *CloudStackIsoNetReconciliationRunner) ReconcileDelete() (retRes ctrl.Re // SetupWithManager sets up the controller with the Manager. func (reconciler *CloudStackIsoNetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). + CloudStackClusterToCloudStackIsolatedNetworks, err := csCtrlrUtils.CloudStackClusterToCloudStackIsolatedNetworks(reconciler.K8sClient, &infrav1.CloudStackIsolatedNetworkList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx)) + if err != nil { + return errors.Wrap(err, "failed to create CloudStackClusterToCloudStackIsolatedNetworks mapper") + } + + err = ctrl.NewControllerManagedBy(mgr). For(&infrav1.CloudStackIsolatedNetwork{}). + Watches( + &infrav1.CloudStackCluster{}, + handler.EnqueueRequestsFromMapFunc(CloudStackClusterToCloudStackIsolatedNetworks), + builder.WithPredicates( + predicate.GenerationChangedPredicate{}, + predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldCSCluster := e.ObjectOld.(*infrav1.CloudStackCluster) + newCSCluster := e.ObjectNew.(*infrav1.CloudStackCluster) + + // APIServerLoadBalancer disabled in both new and old + if oldCSCluster.Spec.APIServerLoadBalancer == nil && newCSCluster.Spec.APIServerLoadBalancer == nil { + return false + } + // APIServerLoadBalancer toggled + if oldCSCluster.Spec.APIServerLoadBalancer == nil || newCSCluster.Spec.APIServerLoadBalancer == nil { + return true + } + + return !reflect.DeepEqual(oldCSCluster.Spec.APIServerLoadBalancer, newCSCluster.Spec.APIServerLoadBalancer) + }, + }, + ), + ). WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), reconciler.WatchFilterValue)). Complete(reconciler) + if err != nil { + return errors.Wrap(err, "failed setting up with a controller manager") + } + + return nil } diff --git a/controllers/cloudstackisolatednetwork_controller_test.go b/controllers/cloudstackisolatednetwork_controller_test.go index 24b9c67a..98bf6feb 100644 --- a/controllers/cloudstackisolatednetwork_controller_test.go +++ b/controllers/cloudstackisolatednetwork_controller_test.go @@ -17,6 +17,7 @@ limitations under the License. package controllers_test import ( + "github.com/apache/cloudstack-go/v2/cloudstack" g "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -38,6 +39,12 @@ var _ = Describe("CloudStackIsolatedNetworkReconciler", func() { It("Should set itself to ready if there are no errors in calls to CloudStack methods.", func() { mockCloudClient.EXPECT().GetOrCreateIsolatedNetwork(g.Any(), g.Any(), g.Any()).AnyTimes() mockCloudClient.EXPECT().AddClusterTag(g.Any(), g.Any(), g.Any()).AnyTimes() + mockCloudClient.EXPECT().AssociatePublicIPAddress(g.Any(), g.Any(), g.Any()).AnyTimes().Return(&cloudstack.PublicIpAddress{ + Id: dummies.PublicIPID, + Associatednetworkid: dummies.ISONet1.ID, + Ipaddress: dummies.CSCluster.Spec.ControlPlaneEndpoint.Host, + }, nil) + mockCloudClient.EXPECT().ReconcileLoadBalancer(g.Any(), g.Any(), g.Any()).AnyTimes() // We use CSFailureDomain2 here because CSFailureDomain1 has an empty Spec.Zone.ID dummies.CSISONet1.Spec.FailureDomainName = dummies.CSFailureDomain2.Spec.Name diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go index 01617476..0aa062bd 100644 --- a/controllers/cloudstackmachine_controller.go +++ b/controllers/cloudstackmachine_controller.go @@ -22,12 +22,14 @@ import ( "math/rand" "reflect" "regexp" + "strings" "time" "k8s.io/utils/pointer" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/cluster-api/util" @@ -226,7 +228,6 @@ func (r *CloudStackMachineReconciliationRunner) GetOrCreateVMInstance() (retRes r.Recorder.Event(r.ReconciliationSubject, "Normal", "Creating", BootstrapDataNotReady) return r.RequeueWithMessage(BootstrapDataNotReady + ".") } - r.Log.Info("Got Bootstrap DataSecretName.") // Get the kubeadm bootstrap secret for this machine. secret := &corev1.Secret{} @@ -268,8 +269,11 @@ func processCustomMetadata(data []byte, r *CloudStackMachineReconciliationRunner // RequeueIfInstanceNotRunning checks the Instance's status for running state and requeues otherwise. func (r *CloudStackMachineReconciliationRunner) RequeueIfInstanceNotRunning() (retRes ctrl.Result, reterr error) { if r.ReconciliationSubject.Status.InstanceState == "Running" { - r.Recorder.Event(r.ReconciliationSubject, "Normal", "Running", MachineInstanceRunning) - r.Log.Info(MachineInstanceRunning) + // Only emit this event when relevant. + if !r.ReconciliationSubject.Status.Ready { + r.Recorder.Event(r.ReconciliationSubject, "Normal", "Running", MachineInstanceRunning) + r.Log.Info(MachineInstanceRunning) + } r.ReconciliationSubject.Status.Ready = true } else if r.ReconciliationSubject.Status.InstanceState == "Error" { r.Recorder.Event(r.ReconciliationSubject, "Warning", "Error", MachineInErrorMessage) @@ -286,39 +290,54 @@ func (r *CloudStackMachineReconciliationRunner) RequeueIfInstanceNotRunning() (r return ctrl.Result{}, nil } -// AddToLBIfNeeded adds instance to load balancer if it is a control plane in an isolated network. +// AddToLBIfNeeded adds instance to load balancer if it is a control plane node in an isolated network, and the load balancer is enabled. func (r *CloudStackMachineReconciliationRunner) AddToLBIfNeeded() (retRes ctrl.Result, reterr error) { - if util.IsControlPlaneMachine(r.CAPIMachine) && r.FailureDomain.Spec.Zone.Network.Type == cloud.NetworkTypeIsolated { - r.Log.Info("Assigning VM to load balancer rule.") + if util.IsControlPlaneMachine(r.CAPIMachine) && + r.FailureDomain.Spec.Zone.Network.Type == cloud.NetworkTypeIsolated && + r.CSCluster.Spec.APIServerLoadBalancer.IsEnabled() { + r.Log.V(4).Info("Assigning VM to load balancer rule.") if r.IsoNet.Spec.Name == "" { return r.RequeueWithMessage("Could not get required Isolated Network for VM, requeueing.") } - err := r.CSUser.AssignVMToLoadBalancerRule(r.IsoNet, *r.ReconciliationSubject.Spec.InstanceID) + err := r.CSUser.AssignVMToLoadBalancerRules(r.IsoNet, *r.ReconciliationSubject.Spec.InstanceID) if err != nil { return ctrl.Result{}, err } } + return ctrl.Result{}, nil } -// GetOrCreateMachineStateChecker creates or gets CloudStackMachineStateChecker object. +// GetOrCreateMachineStateChecker gets or creates a CloudStackMachineStateChecker object. func (r *CloudStackMachineReconciliationRunner) GetOrCreateMachineStateChecker() (retRes ctrl.Result, reterr error) { checkerName := r.ReconciliationSubject.Spec.InstanceID if checkerName == nil { return ctrl.Result{}, errors.New(CSMachineStateCheckerCreationFailed) } - csMachineStateChecker := &infrav1.CloudStackMachineStateChecker{ - ObjectMeta: r.NewChildObjectMeta(*checkerName), - Spec: infrav1.CloudStackMachineStateCheckerSpec{InstanceID: *checkerName}, - Status: infrav1.CloudStackMachineStateCheckerStatus{Ready: false}, - } - if err := r.K8sClient.Create(r.RequestCtx, csMachineStateChecker); err != nil && !utils.ContainsAlreadyExistsSubstring(err) { - r.Recorder.Eventf(r.ReconciliationSubject, "Warning", "Machine State Checker", CSMachineStateCheckerCreationFailed) - return r.ReturnWrappedError(err, CSMachineStateCheckerCreationFailed) + csMachineStateChecker := &infrav1.CloudStackMachineStateChecker{} + objectKey := client.ObjectKey{Name: strings.ToLower(*checkerName), Namespace: r.Request.Namespace} + err := r.K8sClient.Get(r.RequestCtx, objectKey, csMachineStateChecker) + if err != nil { + if k8serrors.IsNotFound(err) { + csMachineStateChecker.ObjectMeta = r.NewChildObjectMeta(*checkerName) + csMachineStateChecker.Spec = infrav1.CloudStackMachineStateCheckerSpec{InstanceID: *checkerName} + csMachineStateChecker.Status = infrav1.CloudStackMachineStateCheckerStatus{Ready: false} + + if err := r.K8sClient.Create(r.RequestCtx, csMachineStateChecker); err != nil { + r.Recorder.Eventf(r.ReconciliationSubject, "Warning", "Machine State Checker", CSMachineStateCheckerCreationFailed) + return r.ReturnWrappedError(err, CSMachineStateCheckerCreationFailed) + } + r.Recorder.Eventf(r.ReconciliationSubject, "Normal", "Machine State Checker", CSMachineStateCheckerCreationSuccess) + } else { + r.Recorder.Eventf(r.ReconciliationSubject, "Warning", "Machine State Checker", CSMachineStateCheckerCreationFailed) + + return ctrl.Result{}, errors.Wrap(err, CSMachineStateCheckerCreationFailed) + } } - r.Recorder.Eventf(r.ReconciliationSubject, "Normal", "Machine State Checker", CSMachineStateCheckerCreationSuccess) - return r.GetObjectByName(*checkerName, r.StateChecker)() + r.StateChecker = csMachineStateChecker + + return ctrl.Result{}, nil } func (r *CloudStackMachineReconciliationRunner) ReconcileDelete() (retRes ctrl.Result, reterr error) { @@ -357,17 +376,19 @@ func (r *CloudStackMachineReconciliationRunner) ReconcileDelete() (retRes ctrl.R // SetupWithManager registers the machine reconciler to the CAPI controller manager. func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts controller.Options) error { + log := ctrl.LoggerFrom(ctx) + reconciler.Recorder = mgr.GetEventRecorderFor("capc-machine-controller") - CloudStackClusterToCloudStackMachines, err := utils.CloudStackClusterToCloudStackMachines(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx)) - if err != nil { - return errors.Wrap(err, "failed to create CloudStackClusterToCloudStackMachines mapper") - } - //requeueCloudStackMachinesForUnpausedCluster := reconciler.requeueCloudStackMachinesForUnpausedCluster(ctx) + + cloudStackClusterToCloudStackMachines := utils.CloudStackClusterToCloudStackMachines(reconciler.K8sClient, log) + csMachineMapper, err := util.ClusterToTypedObjectsMapper(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme) if err != nil { - return errors.Wrap(err, "failed to create mapper for Cluster to AzureMachines") + return errors.Wrap(err, "failed to create mapper for Cluster to CloudStackMachines") } + cloudStackIsolatedNetworkToControlPlaneCloudStackMachines := utils.CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(reconciler.K8sClient, log) + err = ctrl.NewControllerManagedBy(mgr). WithOptions(opts). For(&infrav1.CloudStackMachine{}). @@ -377,7 +398,11 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont builder.WithPredicates( predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - oldMachine := e.ObjectOld.(*clusterv1.Machine) + oldMachine, ok := e.ObjectOld.(*clusterv1.Machine) + if !ok { + log.V(4).Info("Expected Machine", "type", fmt.Sprintf("%T", e.ObjectOld)) + return false + } newMachine := e.ObjectNew.(*clusterv1.Machine) return (oldMachine.Spec.Bootstrap.DataSecretName == nil && newMachine.Spec.Bootstrap.DataSecretName != nil) @@ -387,7 +412,7 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont ). Watches( &infrav1.CloudStackCluster{}, - handler.EnqueueRequestsFromMapFunc(CloudStackClusterToCloudStackMachines), + handler.EnqueueRequestsFromMapFunc(cloudStackClusterToCloudStackMachines), ). WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), reconciler.WatchFilterValue)). WithEventFilter( @@ -434,6 +459,32 @@ func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Cont predicates.ClusterUnpausedAndInfrastructureReady(ctrl.LoggerFrom(ctx)), ), ). + Watches( + // This watch is here to assign VM's to loadbalancer rules + &infrav1.CloudStackIsolatedNetwork{}, + handler.EnqueueRequestsFromMapFunc(cloudStackIsolatedNetworkToControlPlaneCloudStackMachines), + builder.WithPredicates( + predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldCSIsoNet, ok := e.ObjectOld.(*infrav1.CloudStackIsolatedNetwork) + if !ok { + log.V(4).Info("Expected CloudStackIsolatedNetwork", "type", fmt.Sprintf("%T", e.ObjectOld)) + return false + } + + newCSIsoNet := e.ObjectNew.(*infrav1.CloudStackIsolatedNetwork) + + // We're only interested in status updates, not Spec updates + if oldCSIsoNet.Generation != newCSIsoNet.Generation { + return false + } + + // Only trigger a CloudStackMachine reconcile if the loadbalancer rules changed. + return len(oldCSIsoNet.Status.LoadBalancerRuleIDs) != len(newCSIsoNet.Status.LoadBalancerRuleIDs) + }, + }, + ), + ). Complete(reconciler) if err != nil { return errors.Wrap(err, "failed setting up with a controller manager") diff --git a/controllers/controllers_suite_test.go b/controllers/controllers_suite_test.go index d36206f1..fce90ca2 100644 --- a/controllers/controllers_suite_test.go +++ b/controllers/controllers_suite_test.go @@ -22,9 +22,10 @@ import ( "fmt" "go/build" "os" - "os/exec" + "path" "path/filepath" "regexp" + goruntime "runtime" "strings" "testing" "time" @@ -37,7 +38,6 @@ import ( "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -131,16 +131,6 @@ var ( ) var _ = BeforeSuite(func() { - repoRoot := os.Getenv("REPO_ROOT") - - // Add ginkgo recover statements to controllers. - cmd := exec.Command(repoRoot+"/hack/testing_ginkgo_recover_statements.sh", "--contains") - cmd.Stdout = os.Stdout - if err := cmd.Run(); err != nil { - fmt.Println(errors.Wrapf(err, "refusing to run test suite without ginkgo recover statements present")) - os.Exit(1) - } - By("bootstrapping test environment") Ω(infrav1.AddToScheme(scheme.Scheme)).Should(Succeed()) @@ -177,11 +167,14 @@ func (m *MockCtrlrCloudClientImplementation) RegisterExtension(r *csCtrlrUtils.R } func SetupTestEnvironment() { - repoRoot := os.Getenv("REPO_ROOT") - crdPaths := []string{filepath.Join(repoRoot, "config", "crd", "bases"), filepath.Join(repoRoot, "test", "fakes")} + // Get the root of the current file to use in CRD paths. + _, filename, _, _ := goruntime.Caller(0) //nolint:dogsled // Ignore "declaration has 3 blank identifiers" check. + root := path.Join(path.Dir(filename), "..") + + crdPaths := []string{filepath.Join(root, "config", "crd", "bases"), filepath.Join(root, "test", "fakes")} // Append CAPI CRDs path - if capiPath := getFilePathToCAPICRDs(repoRoot); capiPath != "" { + if capiPath := getFilePathToCAPICRDs(root); capiPath != "" { crdPaths = append(crdPaths, capiPath) } testEnv = &envtest.Environment{ diff --git a/controllers/utils/base_reconciler.go b/controllers/utils/base_reconciler.go index 2e1d05b3..b27c5f76 100644 --- a/controllers/utils/base_reconciler.go +++ b/controllers/utils/base_reconciler.go @@ -251,7 +251,7 @@ func (r *ReconciliationRunner) CheckOwnedCRDsForReadiness(gvks ...schema.GroupVe } } -// CheckOwnedObjectsDeleted queries for the presence of owned objects and requeues if any are still present. Primarily +// DeleteOwnedObjects queries for the presence of owned objects and requeues if any are still present. Primarily // used to prevent deletions of owners before dependents. func (r *ReconciliationRunner) DeleteOwnedObjects(gvks ...schema.GroupVersionKind) CloudStackReconcilerMethod { return func() (ctrl.Result, error) { @@ -328,13 +328,6 @@ func (r *ReconciliationRunner) SetupPatcher() (res ctrl.Result, retErr error) { return res, errors.Wrapf(retErr, "setting up patcher") } -// PatchChangesBackToAPI patches changes to the ReconciliationSubject back to the appropriate API. -func (r *ReconciliationRunner) PatchChangesBackToAPI() (res ctrl.Result, retErr error) { - r.Log.V(1).Info("Patching changes back to api.") - err := r.Patcher.Patch(r.RequestCtx, r.ReconciliationSubject) - return res, errors.Wrapf(err, "patching reconciliation subject") -} - // RequeueWithMessage is a convenience method to log requeue message and then return a result with RequeueAfter set. func (r *ReconciliationRunner) RequeueWithMessage(msg string, keysAndValues ...interface{}) (ctrl.Result, error) { // Add requeuing to message if not present. Might turn this into a lint check later. @@ -350,17 +343,12 @@ func (r *ReconciliationRunner) ReturnWrappedError(err error, msg string) (ctrl.R return ctrl.Result{}, errors.Wrap(err, msg) } -func (r *ReconciliationRunner) LogReconciliationSubject() (ctrl.Result, error) { - r.Log.Info("The subject", "subject", r.ReconciliationSubject) - return ctrl.Result{}, nil -} - // CloudStackReconcilerMethod is the method type used in RunReconciliationStages. Additional arguments can be added // by wrapping this type in another function affectively currying them. type CloudStackReconcilerMethod func() (ctrl.Result, error) -// RunReconciliationStage runs a CloudStackReconcilerMethod and returns a boolean to indicate whether that stage would -// have returned a result that cuts the process short or not. +// ShouldReturn returns a boolean to indicate whether a CloudStackReconcilerMethod returned a result that should cut the +// reconciliation process short or not. func (r *ReconciliationRunner) ShouldReturn(rslt ctrl.Result, err error) bool { if err != nil { return true @@ -380,6 +368,9 @@ func (r *ReconciliationRunner) RunReconciliationStages(fns ...CloudStackReconcil return rslt, nil } } + + r.Log.V(1).Info("Finished reconciliation") + return ctrl.Result{}, nil } @@ -429,7 +420,7 @@ func (r *ReconciliationRunner) SetReturnEarly() { r.returnEarly = true } -// GetReconcilationSubject gets the reconciliation subject of type defined by the concrete reconciler. It also sets up +// GetReconciliationSubject gets the reconciliation subject of type defined by the concrete reconciler. It also sets up // a patch helper at this point. func (r *ReconciliationRunner) GetReconciliationSubject() (res ctrl.Result, reterr error) { r.Log.V(1).Info("Getting reconciliation subject.") @@ -500,7 +491,6 @@ func (r *ReconciliationRunner) GetObjectByName(name string, target client.Object if len(nameGetter) == 1 { name = nameGetter[0]() } - name = strings.ToLower(name) objectKey := client.ObjectKey{Name: strings.ToLower(name), Namespace: r.Request.Namespace} return r.ReturnWrappedError( client.IgnoreNotFound(r.K8sClient.Get(r.RequestCtx, objectKey, target)), "failed to get object") diff --git a/controllers/utils/isolated_network.go b/controllers/utils/isolated_network.go index ef91e0ed..3c6bc018 100644 --- a/controllers/utils/isolated_network.go +++ b/controllers/utils/isolated_network.go @@ -32,17 +32,20 @@ func (r *ReconciliationRunner) IsoNetMetaName(name string) string { return strings.TrimSuffix(str, "-") } -// GenerateIsolatedNetwork of the passed name that's owned by the ReconciliationSubject. -func (r *ReconciliationRunner) GenerateIsolatedNetwork(name, fdName, domainName string) CloudStackReconcilerMethod { +// GenerateIsolatedNetwork creates a CloudStackIsolatedNetwork object that is owned by the ReconciliationSubject. +func (r *ReconciliationRunner) GenerateIsolatedNetwork(network infrav1.Network, fdName string) CloudStackReconcilerMethod { return func() (ctrl.Result, error) { - lowerName := strings.ToLower(name) + lowerName := strings.ToLower(network.Name) metaName := fmt.Sprintf("%s-%s", r.CSCluster.Name, lowerName) csIsoNet := &infrav1.CloudStackIsolatedNetwork{} csIsoNet.ObjectMeta = r.NewChildObjectMeta(metaName) csIsoNet.Spec.Name = lowerName csIsoNet.Spec.FailureDomainName = fdName - if domainName != "" { - csIsoNet.Spec.Domain = strings.ToLower(domainName) + if network.CIDR != "" { + csIsoNet.Spec.CIDR = network.CIDR + } + if network.Domain != "" { + csIsoNet.Spec.Domain = strings.ToLower(network.Domain) } csIsoNet.Spec.ControlPlaneEndpoint.Host = r.CSCluster.Spec.ControlPlaneEndpoint.Host csIsoNet.Spec.ControlPlaneEndpoint.Port = r.CSCluster.Spec.ControlPlaneEndpoint.Port diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index c591b9c1..cbeee225 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -21,6 +21,9 @@ import ( "fmt" "strings" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/go-logr/logr" "github.com/pkg/errors" @@ -111,21 +114,74 @@ func GetOwnerClusterName(obj metav1.ObjectMeta) (string, error) { return "", errors.New("failed to get owner Cluster name") } -// CloudStackClusterToCloudStackMachines is a handler.ToRequestsFunc to be used to enqeue requests for reconciliation +// CloudStackClusterToCloudStackMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation // of CloudStackMachines. -func CloudStackClusterToCloudStackMachines(c client.Client, obj runtime.Object, scheme *runtime.Scheme, log logr.Logger) (handler.MapFunc, error) { +func CloudStackClusterToCloudStackMachines(c client.Client, log logr.Logger) handler.MapFunc { + return func(ctx context.Context, o client.Object) []reconcile.Request { + csCluster, ok := o.(*infrav1.CloudStackCluster) + if !ok { + log.Error(fmt.Errorf("expected a CloudStackCluster but got a %T", o), "Error in CloudStackClusterToCloudStackMachines") + return nil + } + + log := log.WithValues("objectMapper", "cloudstackClusterToCloudStackMachine", "cluster", klog.KRef(csCluster.Namespace, csCluster.Name)) + + // Don't handle deleted CloudStackClusters + if !csCluster.ObjectMeta.DeletionTimestamp.IsZero() { + log.V(4).Info("CloudStackCluster has a deletion timestamp, skipping mapping.") + return nil + } + + clusterName, err := GetOwnerClusterName(csCluster.ObjectMeta) + if err != nil { + log.Error(err, "Failed to get owning cluster, skipping mapping.") + return nil + } + + machineList := &clusterv1.MachineList{} + // list all the requested objects within the cluster namespace with the cluster name label + if err := c.List(ctx, machineList, client.InNamespace(csCluster.Namespace), client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName}); err != nil { + log.Error(err, "Failed to get owned Machines, skipping mapping.") + return nil + } + + results := make([]ctrl.Request, 0, len(machineList.Items)) + for _, machine := range machineList.Items { + m := machine + log.WithValues("machine", klog.KObj(&m)) + if m.Spec.InfrastructureRef.GroupVersionKind().Kind != "CloudStackMachine" { + log.V(4).Info("Machine has an InfrastructureRef for a different type, will not add to reconciliation request.") + continue + } + if m.Spec.InfrastructureRef.Name == "" { + log.V(4).Info("Machine has an InfrastructureRef with an empty name, will not add to reconciliation request.") + continue + } + log.WithValues("cloudStackMachine", klog.KRef(m.Spec.InfrastructureRef.Namespace, m.Spec.InfrastructureRef.Name)) + log.V(4).Info("Adding CloudStackMachine to reconciliation request.") + results = append(results, ctrl.Request{NamespacedName: client.ObjectKey{Namespace: m.Namespace, Name: m.Spec.InfrastructureRef.Name}}) + } + + return results + } +} + +// CloudStackClusterToCloudStackIsolatedNetworks is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation +// of CloudStackIsolatedNetworks. +func CloudStackClusterToCloudStackIsolatedNetworks(c client.Client, obj client.ObjectList, scheme *runtime.Scheme, log logr.Logger) (handler.MapFunc, error) { gvk, err := apiutil.GVKForObject(obj, scheme) if err != nil { - return nil, errors.Wrap(err, "failed to find GVK for CloudStackMachine") + return nil, errors.Wrap(err, "failed to find GVK for CloudStackIsolatedNetwork") } - return func(ctx context.Context, o client.Object) []ctrl.Request { + return func(ctx context.Context, o client.Object) []reconcile.Request { csCluster, ok := o.(*infrav1.CloudStackCluster) if !ok { - log.Error(fmt.Errorf("expected a CloudStackCluster but got a %T", o), "Error in CloudStackClusterToCloudStackMachines") + log.Error(fmt.Errorf("expected a CloudStackCluster but got a %T", o), "Error in CloudStackClusterToCloudStackIsolatedNetworks") + return nil } - log = log.WithValues("objectMapper", "cloudstackClusterToCloudStackMachine", "cluster", klog.KRef(csCluster.Namespace, csCluster.Name)) + log = log.WithValues("objectMapper", "cloudstackClusterToCloudStackIsolatedNetworks", "cluster", klog.KRef(csCluster.Namespace, csCluster.Name)) // Don't handle deleted CloudStackClusters if !csCluster.ObjectMeta.DeletionTimestamp.IsZero() { @@ -139,23 +195,84 @@ func CloudStackClusterToCloudStackMachines(c client.Client, obj runtime.Object, return nil } + isonetList := &infrav1.CloudStackIsolatedNetworkList{} + isonetList.SetGroupVersionKind(gvk) + + // list all the requested objects within the cluster namespace with the cluster name label + if err := c.List(ctx, isonetList, client.InNamespace(csCluster.Namespace), client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName}); err != nil { + return nil + } + + var results []reconcile.Request + for _, isonet := range isonetList.Items { + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: isonet.GetNamespace(), + Name: isonet.GetName(), + }, + } + results = append(results, req) + } + + return results + }, nil +} + +// CloudStackIsolatedNetworkToControlPlaneCloudStackMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation +// of CloudStackMachines that are part of the control plane. +func CloudStackIsolatedNetworkToControlPlaneCloudStackMachines(c client.Client, log logr.Logger) handler.MapFunc { + return func(ctx context.Context, o client.Object) []reconcile.Request { + csIsoNet, ok := o.(*infrav1.CloudStackIsolatedNetwork) + if !ok { + log.Error(fmt.Errorf("expected a CloudStackIsolatedNetwork but got a %T", o), "Error in CloudStackIsolatedNetworkToControlPlaneCloudStackMachines") + return nil + } + + log := log.WithValues("objectMapper", "cloudStackIsolatedNetworkToControlPlaneCloudStackMachines", "isonet", klog.KRef(csIsoNet.Namespace, csIsoNet.Name)) + + // Don't handle deleted CloudStackIsolatedNetworks + if !csIsoNet.ObjectMeta.DeletionTimestamp.IsZero() { + log.V(4).Info("CloudStackIsolatedNetwork has a deletion timestamp, skipping mapping.") + return nil + } + + clusterName, ok := csIsoNet.GetLabels()[clusterv1.ClusterNameLabel] + if !ok { + log.Error(errors.New("failed to find cluster name label"), "CloudStackIsolatedNetwork is missing cluster name label or cluster does not exist, skipping mapping.") + } + machineList := &clusterv1.MachineList{} - machineList.SetGroupVersionKind(gvk) - // list all of the requested objects within the cluster namespace with the cluster name label - if err := c.List(ctx, machineList, client.InNamespace(csCluster.Namespace), client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName}); err != nil { + // list all the requested objects within the cluster namespace with the cluster name and control plane label. + err := c.List(ctx, machineList, client.InNamespace(csIsoNet.Namespace), client.MatchingLabels{ + clusterv1.ClusterNameLabel: clusterName, + clusterv1.MachineControlPlaneLabel: "", + }) + if err != nil { + log.Error(err, "Failed to get owned control plane Machines, skipping mapping.") return nil } - mapFunc := util.MachineToInfrastructureMapFunc(gvk) - var results []ctrl.Request + log.V(4).Info("Looked up members with control plane label", "found", len(machineList.Items)) + + results := make([]ctrl.Request, 0, len(machineList.Items)) for _, machine := range machineList.Items { m := machine - csMachines := mapFunc(ctx, &m) - results = append(results, csMachines...) + log.WithValues("machine", klog.KObj(&m)) + if m.Spec.InfrastructureRef.GroupVersionKind().Kind != "CloudStackMachine" { + log.V(4).Info("Machine has an InfrastructureRef for a different type, will not add to reconciliation request.") + continue + } + if m.Spec.InfrastructureRef.Name == "" { + log.V(4).Info("Machine has an InfrastructureRef with an empty name, will not add to reconciliation request.") + continue + } + log.WithValues("cloudStackMachine", klog.KRef(m.Spec.InfrastructureRef.Namespace, m.Spec.InfrastructureRef.Name)) + log.V(4).Info("Adding CloudStackMachine to reconciliation request.") + results = append(results, ctrl.Request{NamespacedName: client.ObjectKey{Namespace: m.Namespace, Name: m.Spec.InfrastructureRef.Name}}) } return results - }, nil + } } // DebugPredicate returns a predicate that logs the event that triggered the reconciliation diff --git a/go.mod b/go.mod index 4df5022e..f5344497 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module sigs.k8s.io/cluster-api-provider-cloudstack -go 1.21 +go 1.22 require ( github.com/apache/cloudstack-go/v2 v2.16.1 @@ -16,10 +16,10 @@ require ( github.com/spf13/pflag v1.0.5 golang.org/x/text v0.16.0 gopkg.in/yaml.v3 v3.0.1 - k8s.io/api v0.27.14 - k8s.io/apimachinery v0.27.14 - k8s.io/client-go v0.27.14 - k8s.io/component-base v0.27.14 + k8s.io/api v0.27.16 + k8s.io/apimachinery v0.27.16 + k8s.io/client-go v0.27.16 + k8s.io/component-base v0.27.16 k8s.io/klog/v2 v2.90.1 k8s.io/utils v0.0.0-20230505201702-9f6742963106 sigs.k8s.io/cluster-api v1.5.8 @@ -76,8 +76,8 @@ require ( google.golang.org/protobuf v1.33.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - k8s.io/apiextensions-apiserver v0.27.14 // indirect - k8s.io/cluster-bootstrap v0.27.14 // indirect + k8s.io/apiextensions-apiserver v0.27.16 // indirect + k8s.io/cluster-bootstrap v0.27.16 // indirect k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect diff --git a/go.sum b/go.sum index f80572d4..935f6d6d 100644 --- a/go.sum +++ b/go.sum @@ -362,20 +362,20 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -k8s.io/api v0.27.14 h1:/oKAF9HiSB47polol2Ji2TaFnC400JK57jSPUXY5MzU= -k8s.io/api v0.27.14/go.mod h1:Jekhd9Kyo2CsmJlYbqZPXNwIxiHvyGJCdp0X56yDyvU= -k8s.io/apiextensions-apiserver v0.27.14 h1:d7GtKY9F0drdgAxLnKHgAzUBXQVqyDlkjDHUHXr4B2w= -k8s.io/apiextensions-apiserver v0.27.14/go.mod h1:+i082aG+ZX89DlApNxVWj8U0VqkbvHmTF/KjpzzHhLU= -k8s.io/apimachinery v0.27.14 h1:jAIGvPbvAg4XJysK7JPFa6DdjTR6vts4/p4Q6ZrcQ+4= -k8s.io/apimachinery v0.27.14/go.mod h1:TWo+8wOIz3CytsrlI9k/LBWXLRr9dqf5hRSCbbggMAg= -k8s.io/apiserver v0.27.14 h1:qBCxFtCjKlokT6Bn6YtjvfiquU/3cOA4Pv0Yqw5BsiM= -k8s.io/apiserver v0.27.14/go.mod h1:/QZ7+YwuOuybRvRWamIUUZnYnb1SHHSAlLFlmaZUrvM= -k8s.io/client-go v0.27.14 h1:5KwfSakOTQFRlPru2Ql/wp1URjPgzoP7QpTlEH9a+ys= -k8s.io/client-go v0.27.14/go.mod h1:cy+p3ijvbPQpdcwg01qnHBmkYDtbOatNC83anA9y18g= -k8s.io/cluster-bootstrap v0.27.14 h1:Ue0r2H9KwspOHfWaKW2AukgrRwR7Ev1JfyvzcrfLCo8= -k8s.io/cluster-bootstrap v0.27.14/go.mod h1:uy3hxFCNPLCfC3jiSqtsngeOpp72BHDJ3Pk3NVzJhjI= -k8s.io/component-base v0.27.14 h1:Pdl5bL1TX/MtXsIwUtSctccgZRmrao6GCV3+qfxw0Qs= -k8s.io/component-base v0.27.14/go.mod h1:eiXBPnqBoczGNS09AtEWxN3Bpj4mI32FGb54S+JAcPg= +k8s.io/api v0.27.16 h1:70IBoTuiPfd+Tm68WH0tGXQRSQq0R1xnbyhTRe8WYQY= +k8s.io/api v0.27.16/go.mod h1:5j0Cgo6X4qovBOu3OjzRwETDEYqMxq2qafhDQXOPy3A= +k8s.io/apiextensions-apiserver v0.27.16 h1:gJ0sEbfYmvgdysC2WjkeYujvjmWAyPH6e8ANVAL5qxk= +k8s.io/apiextensions-apiserver v0.27.16/go.mod h1:wq5IgoFVjYyJqqcjD+R+/opZJxBQcu9PIcFWJ8eaQLQ= +k8s.io/apimachinery v0.27.16 h1:Nmbei3P/6w6vxbNxV8/sDCZz+TQrJ9A4+bVIRjDufuM= +k8s.io/apimachinery v0.27.16/go.mod h1:TWo+8wOIz3CytsrlI9k/LBWXLRr9dqf5hRSCbbggMAg= +k8s.io/apiserver v0.27.16 h1:s3+lMqISTj5l/ZH/BvhdbiMfIoTF3/lrAN99BHccLmk= +k8s.io/apiserver v0.27.16/go.mod h1:xwxM8/bcAtgkWqbsGwMQjImIC5Jik7a4pHRptEDqQf0= +k8s.io/client-go v0.27.16 h1:x06Jk6/SIQQ6kAsWs5uzQIkBLHtcAQlbTAgmj1tZzG0= +k8s.io/client-go v0.27.16/go.mod h1:bPZUNRj8XsHa+JVS5jU6qeU2H/Za8+7riWA08FUjaA8= +k8s.io/cluster-bootstrap v0.27.16 h1:iySGnST9X4W1IfAdANdF6uBzV6kTL9SIAiKQnlZm4ug= +k8s.io/cluster-bootstrap v0.27.16/go.mod h1:u7tVB3+r4X0I/fEfirH+dNWuwGjQyReAVZa7/V92Pkk= +k8s.io/component-base v0.27.16 h1:CpPBD1GIwsaRdDF0WzJkIppakYJwQCvsKK8exRxe9rY= +k8s.io/component-base v0.27.16/go.mod h1:g636fljq9A7zsIB0nRE4fgmBCo8aqjoJe1aLkCX0Vwc= k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw= k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5FJ2kxm1WrQFanWchyKuqGg= diff --git a/hack/testing_ginkgo_recover_statements.sh b/hack/testing_ginkgo_recover_statements.sh deleted file mode 100755 index 17a9c77e..00000000 --- a/hack/testing_ginkgo_recover_statements.sh +++ /dev/null @@ -1,48 +0,0 @@ -#!/bin/bash - -# Copyright 2022. -# -# Licensed 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 is a simple script to assist in adding GinkgoRecover() statements to controllers during testing. -# This is necessary as the controllers are run in goroutines when tested under testenv. -# Add to add, remove to remove, and contains exits 1 if the statements are missing. - -CONTROLLER_DIR=${REPO_ROOT:-$(dirname $(dirname "$0"))}/controllers -FILES=${CONTROLLER_DIR}/cloudstack*controller.go - -case $1 in - --add) - # Use grep to prevent double addition of ginkgo recover statements. - grep -i ginkgo ${FILES} 2>&1> /dev/null \ - || (sed -i.bak '/Reconcile(/a\'$'\n'$'\t''defer ginkgo.GinkgoRecover()'$'\n''' ${FILES} && \ - sed -i.bak '/^import (/a\'$'\n'$'\t''"github.com/onsi/ginkgo/v2"'$'\n''' ${FILES} && \ - rm ${CONTROLLER_DIR}/*.bak) - ;; - --remove) - sed -i.bak '/ginkgo/d' ${FILES} && rm ${CONTROLLER_DIR}/*.bak - ;; - --contains) - grep -i ginkgo ${FILES} 2>&1> /dev/null && exit 0 - echo "**************************************************************************************************************" - echo "******************************************************************************************************************" - echo "Did not find GinkgoRecover statements present in controllers." - echo "Please run $0 " - echo "with the '--add' argument to add to tests." - echo "Without this, controller test failures will result in a Ginkgo Panic," echo "and failures will be opaque." - echo "******************************************************************************************************************" - echo "**************************************************************************************************************" - exit 1 - ;; -esac - diff --git a/hack/tools/go.mod b/hack/tools/go.mod index b720d4b3..9155406a 100644 --- a/hack/tools/go.mod +++ b/hack/tools/go.mod @@ -1,6 +1,6 @@ module sigs.k8s.io/cluster-api-provider-cloudstack/hack/tools -go 1.21 +go 1.22 require sigs.k8s.io/cluster-api/hack/tools v0.0.0-20240311182002-eeab3ceb5ecc diff --git a/hack/version.sh b/hack/version.sh new file mode 100755 index 00000000..0dc888ac --- /dev/null +++ b/hack/version.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# Copyright 2020 The Kubernetes Authors. +# +# Licensed 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. + +set -o errexit +set -o nounset +set -o pipefail + +version::get_version_vars() { + # shellcheck disable=SC1083 + GIT_COMMIT="$(git rev-parse HEAD^{commit})" + + if git_status=$(git status --porcelain 2>/dev/null) && [[ -z ${git_status} ]]; then + GIT_TREE_STATE="clean" + else + GIT_TREE_STATE="dirty" + fi + + # stolen from k8s.io/hack/lib/version.sh + # Use git describe to find the version based on tags. + if GIT_VERSION=$(git describe --tags --abbrev=14 2>/dev/null); then + # This translates the "git describe" to an actual semver.org + # compatible semantic version that looks something like this: + # v1.1.0-alpha.0.6+84c76d1142ea4d + # + # TODO: We continue calling this "git version" because so many + # downstream consumers are expecting it there. + # shellcheck disable=SC2001 + DASHES_IN_VERSION=$(echo "${GIT_VERSION}" | sed "s/[^-]//g") + if [[ "${DASHES_IN_VERSION}" == "---" ]] ; then + # We have distance to subversion (v1.1.0-subversion-1-gCommitHash) + # shellcheck disable=SC2001 + GIT_VERSION=$(echo "${GIT_VERSION}" | sed "s/-\([0-9]\{1,\}\)-g\([0-9a-f]\{14\}\)$/.\1\-\2/") + elif [[ "${DASHES_IN_VERSION}" == "--" ]] ; then + # We have distance to base tag (v1.1.0-1-gCommitHash) + # shellcheck disable=SC2001 + GIT_VERSION=$(echo "${GIT_VERSION}" | sed "s/-g\([0-9a-f]\{14\}\)$/-\1/") + fi + if [[ "${GIT_TREE_STATE}" == "dirty" ]]; then + # git describe --dirty only considers changes to existing files, but + # that is problematic since new untracked .go files affect the build, + # so use our idea of "dirty" from git status instead. + GIT_VERSION+="-dirty" + fi + + + # Try to match the "git describe" output to a regex to try to extract + # the "major" and "minor" versions and whether this is the exact tagged + # version or whether the tree is between two tagged versions. + if [[ "${GIT_VERSION}" =~ ^v([0-9]+)\.([0-9]+)(\.[0-9]+)?([-].*)?([+].*)?$ ]]; then + GIT_MAJOR=${BASH_REMATCH[1]} + GIT_MINOR=${BASH_REMATCH[2]} + fi + + # If GIT_VERSION is not a valid Semantic Version, then refuse to build. + if ! [[ "${GIT_VERSION}" =~ ^v([0-9]+)\.([0-9]+)(\.[0-9]+)?(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$ ]]; then + echo "GIT_VERSION should be a valid Semantic Version. Current value: ${GIT_VERSION}" + echo "Please see more details here: https://semver.org" + exit 1 + fi + fi + + GIT_RELEASE_TAG=$(git describe --abbrev=0 --tags) + GIT_RELEASE_COMMIT=$(git rev-list -n 1 "${GIT_RELEASE_TAG}") +} + +# stolen from k8s.io/hack/lib/version.sh and modified +# Prints the value that needs to be passed to the -ldflags parameter of go build +version::ldflags() { + version::get_version_vars + + local -a ldflags + function add_ldflag() { + local key=${1} + local val=${2} + ldflags+=( + "-X 'sigs.k8s.io/cluster-api-provider-cloudstack/version.${key}=${val}'" + ) + } + + add_ldflag "buildDate" "$(date ${SOURCE_DATE_EPOCH:+"--date=@${SOURCE_DATE_EPOCH}"} -u +'%Y-%m-%dT%H:%M:%SZ')" + add_ldflag "gitCommit" "${GIT_COMMIT}" + add_ldflag "gitTreeState" "${GIT_TREE_STATE}" + add_ldflag "gitMajor" "${GIT_MAJOR}" + add_ldflag "gitMinor" "${GIT_MINOR}" + add_ldflag "gitVersion" "${GIT_VERSION}" + add_ldflag "gitReleaseCommit" "${GIT_RELEASE_COMMIT}" + + # The -ldflags parameter takes a single string, so join the output. + echo "${ldflags[*]-}" +} + +version::ldflags diff --git a/main.go b/main.go index c3b2c870..760ba617 100644 --- a/main.go +++ b/main.go @@ -18,15 +18,17 @@ package main import ( "context" + "flag" "fmt" "k8s.io/klog/v2" + "net/http" "os" + "sigs.k8s.io/cluster-api-provider-cloudstack/version" + "time" - flag "github.com/spf13/pflag" + "github.com/spf13/pflag" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - goflag "flag" - corev1 "k8s.io/api/core/v1" _ "k8s.io/client-go/plugin/pkg/client/auth" cgrecord "k8s.io/client-go/tools/record" @@ -57,9 +59,6 @@ import ( var ( scheme = runtime.NewScheme() setupLog = ctrl.Log.WithName("setup") - - tlsOptions = flags.TLSOptions{} - logOptions = logs.NewOptions() ) func init() { @@ -71,128 +70,163 @@ func init() { //+kubebuilder:scaffold:scheme } -type managerOpts struct { - CloudConfigFile string - MetricsAddr string - EnableLeaderElection bool - ProbeAddr string - WatchNamespace string - WatchFilterValue string - ProfilerAddr string - WebhookCertDir string - WebhookPort int - - CloudStackClusterConcurrency int - CloudStackMachineConcurrency int - CloudStackAffinityGroupConcurrency int - CloudStackFailureDomainConcurrency int -} +var ( + enableLeaderElection bool + leaderElectionLeaseDuration time.Duration + leaderElectionRenewDeadline time.Duration + leaderElectionRetryPeriod time.Duration + leaderElectionNamespace string + watchNamespace string + watchFilterValue string + profilerAddr string + metricsAddr string + probeAddr string + syncPeriod time.Duration + webhookCertDir string + webhookPort int + showVersion bool + + cloudStackClusterConcurrency int + cloudStackMachineConcurrency int + cloudStackAffinityGroupConcurrency int + cloudStackFailureDomainConcurrency int + + tlsOptions = flags.TLSOptions{} + logOptions = logs.NewOptions() +) -func setFlags() *managerOpts { - opts := &managerOpts{} - flag.StringVar( - &opts.CloudConfigFile, - "cloud-config-file", - "/config/cloud-config", - "Overrides the default path to the cloud-config file that contains the CloudStack credentials.") - flag.StringVar( - &opts.MetricsAddr, +func initFlags(fs *pflag.FlagSet) { + fs.StringVar( + &metricsAddr, "metrics-bind-addr", "localhost:8080", "The address the metric endpoint binds to.") - flag.StringVar( - &opts.ProbeAddr, + fs.StringVar( + &probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") - flag.BoolVar( - &opts.EnableLeaderElection, + fs.BoolVar( + &enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") - flag.StringVar( - &opts.WatchNamespace, + fs.DurationVar(&leaderElectionLeaseDuration, "leader-elect-lease-duration", 15*time.Second, + "Interval at which non-leader candidates will wait to force acquire leadership (duration string)") + + fs.DurationVar(&leaderElectionRenewDeadline, "leader-elect-renew-deadline", 10*time.Second, + "Duration that the leading controller manager will retry refreshing leadership before giving up (duration string)") + + fs.DurationVar(&leaderElectionRetryPeriod, "leader-elect-retry-period", 2*time.Second, + "Duration the LeaderElector clients should wait between tries of actions (duration string)") + fs.StringVar( + &watchNamespace, "namespace", "", "Namespace that the controller watches to reconcile cluster-api objects. If unspecified, "+ "the controller watches for cluster-api objects across all namespaces.") - flag.StringVar( - &opts.WatchFilterValue, + fs.StringVar( + &watchFilterValue, "watch-filter", "", fmt.Sprintf( "Label value that the controller watches to reconcile cluster-api objects. "+ "Label key is always %s. If unspecified, the controller watches for all cluster-api objects.", clusterv1.WatchLabel)) - flag.StringVar( - &opts.ProfilerAddr, + fs.StringVar( + &leaderElectionNamespace, + "leader-elect-namespace", + "", + "Namespace that the controller performs leader election in. If unspecified, the controller will discover which namespace it is running in.", + ) + fs.StringVar( + &profilerAddr, "profiler-address", "", "Bind address to expose the pprof profiler (e.g. localhost:6060)") - flag.IntVar( - &opts.WebhookPort, + fs.IntVar( + &webhookPort, "webhook-port", 9443, "The webhook server port the manager will listen on.") - flag.StringVar( - &opts.WebhookCertDir, + fs.StringVar( + &webhookCertDir, "webhook-cert-dir", "/tmp/k8s-webhook-server/serving-certs/", "Specify the directory where webhooks will get tls certificates.") - flag.IntVar( - &opts.CloudStackClusterConcurrency, + fs.IntVar( + &cloudStackClusterConcurrency, "cloudstackcluster-concurrency", 10, "Maximum concurrent reconciles for CloudStackCluster resources", ) - flag.IntVar( - &opts.CloudStackMachineConcurrency, + fs.IntVar( + &cloudStackMachineConcurrency, "cloudstackmachine-concurrency", 10, "Maximum concurrent reconciles for CloudStackMachine resources", ) - flag.IntVar( - &opts.CloudStackAffinityGroupConcurrency, + fs.IntVar( + &cloudStackAffinityGroupConcurrency, "cloudstackaffinitygroup-concurrency", 5, "Maximum concurrent reconciles for CloudStackAffinityGroup resources", ) - flag.IntVar( - &opts.CloudStackFailureDomainConcurrency, + fs.IntVar( + &cloudStackFailureDomainConcurrency, "cloudstackfailuredomain-concurrency", 5, "Maximum concurrent reconciles for CloudStackFailureDomain resources", ) + fs.DurationVar(&syncPeriod, + "sync-period", + 10*time.Minute, + "The minimum interval at which watched resources are reconciled", + ) + fs.BoolVar(&showVersion, "version", false, "Show current version and exit.") - return opts + logs.AddFlags(fs, logs.SkipLoggingConfigurationFlags()) + logsv1.AddFlags(logOptions, fs) + + flags.AddTLSOptions(fs, &tlsOptions) } func main() { - opts := setFlags() // Add our options to flag set. - logsv1.AddFlags(logOptions, flag.CommandLine) - flags.AddTLSOptions(flag.CommandLine, &tlsOptions) - flag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - flag.CommandLine.AddGoFlagSet(goflag.CommandLine) // Merge klog's goflag flags into the pflags. - flag.Parse() + initFlags(pflag.CommandLine) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) + pflag.CommandLine.AddGoFlagSet(flag.CommandLine) // Merge klog's goflag flags into the pflags. + pflag.Parse() + + if showVersion { + fmt.Println(version.Get().String()) //nolint:forbidigo + os.Exit(0) + } if err := logsv1.ValidateAndApply(logOptions, nil); err != nil { setupLog.Error(err, "unable to start manager") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } ctrl.SetLogger(klog.Background()) + if profilerAddr != "" { + klog.Infof("Profiler listening for requests at %s", profilerAddr) + go func() { + klog.Info(http.ListenAndServe(profilerAddr, nil)) //nolint:gosec + }() + } + tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) if err != nil { setupLog.Error(err, "unable to add TLS settings to the webhook server") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } var watchNamespaces []string - if opts.WatchNamespace != "" { - setupLog.Info("Watching cluster-api objects only in namespace for reconciliation", "namespace", opts.WatchNamespace) - watchNamespaces = []string{opts.WatchNamespace} + if watchNamespace != "" { + setupLog.Info("Watching cluster-api objects only in namespace for reconciliation", "namespace", watchNamespace) + watchNamespaces = []string{watchNamespace} } // Machine and cluster operations can create enough events to trigger the event recorder spam filter @@ -207,15 +241,19 @@ func main() { // Create the controller manager. mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: scheme, - MetricsBindAddress: opts.MetricsAddr, - Port: 9443, - HealthProbeBindAddress: opts.ProbeAddr, - LeaderElection: opts.EnableLeaderElection, - LeaderElectionID: "capc-leader-election-controller", - PprofBindAddress: opts.ProfilerAddr, + Scheme: scheme, + MetricsBindAddress: metricsAddr, + HealthProbeBindAddress: probeAddr, + LeaderElection: enableLeaderElection, + LeaderElectionID: "capc-leader-election-controller", + LeaderElectionNamespace: leaderElectionNamespace, + LeaseDuration: &leaderElectionLeaseDuration, + RenewDeadline: &leaderElectionRenewDeadline, + RetryPeriod: &leaderElectionRetryPeriod, + PprofBindAddress: profilerAddr, Cache: cache.Options{ Namespaces: watchNamespaces, + SyncPeriod: &syncPeriod, }, Client: client.Options{ Cache: &client.CacheOptions{ @@ -226,15 +264,15 @@ func main() { }, }, WebhookServer: webhook.NewServer(webhook.Options{ - Port: opts.WebhookPort, - CertDir: opts.WebhookCertDir, + Port: webhookPort, + CertDir: webhookCertDir, TLSOpts: tlsOptionOverrides, }), EventBroadcaster: broadcaster, }) if err != nil { setupLog.Error(err, "unable to start manager") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } // Register reconcilers with the controller manager. @@ -242,11 +280,11 @@ func main() { K8sClient: mgr.GetClient(), Recorder: mgr.GetEventRecorderFor("capc-controller-manager"), Scheme: mgr.GetScheme(), - WatchFilterValue: opts.WatchFilterValue, + WatchFilterValue: watchFilterValue, } ctx := ctrl.SetupSignalHandler() - setupReconcilers(ctx, base, *opts, mgr) + setupReconcilers(ctx, base, mgr) infrav1b3.K8sClient = base.K8sClient // +kubebuilder:scaffold:builder @@ -254,53 +292,53 @@ func main() { // Add health and ready checks. if err = mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } if err = mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up ready check") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } // Start the controller manager. if err = (&infrav1b3.CloudStackCluster{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "CloudStackCluster") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } if err = (&infrav1b3.CloudStackMachine{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "CloudStackMachine") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } if err = (&infrav1b3.CloudStackMachineTemplate{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "CloudStackMachineTemplate") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } - setupLog.Info("starting manager") + setupLog.Info("starting manager", "version", version.Get().String()) if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "problem running manager") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } } -func setupReconcilers(ctx context.Context, base utils.ReconcilerBase, opts managerOpts, mgr manager.Manager) { - if err := (&controllers.CloudStackClusterReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: opts.CloudStackClusterConcurrency}); err != nil { +func setupReconcilers(ctx context.Context, base utils.ReconcilerBase, mgr manager.Manager) { + if err := (&controllers.CloudStackClusterReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: cloudStackClusterConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CloudStackCluster") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } - if err := (&controllers.CloudStackMachineReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: opts.CloudStackMachineConcurrency}); err != nil { + if err := (&controllers.CloudStackMachineReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: cloudStackMachineConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CloudStackMachine") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } if err := (&controllers.CloudStackIsoNetReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CloudStackIsoNetReconciler") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } - if err := (&controllers.CloudStackAffinityGroupReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: opts.CloudStackAffinityGroupConcurrency}); err != nil { + if err := (&controllers.CloudStackAffinityGroupReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: cloudStackAffinityGroupConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CloudStackAffinityGroup") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } - if err := (&controllers.CloudStackFailureDomainReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: opts.CloudStackFailureDomainConcurrency}); err != nil { + if err := (&controllers.CloudStackFailureDomainReconciler{ReconcilerBase: base}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: cloudStackFailureDomainConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CloudStackFailureDomain") - os.Exit(1) + klog.FlushAndExit(klog.ExitFlushTimeout, 1) } } diff --git a/metadata.yaml b/metadata.yaml index bd61395c..caaadcbc 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -6,6 +6,12 @@ apiVersion: clusterctl.cluster.x-k8s.io/v1alpha3 kind: Metadata releaseSeries: + - major: 0 + minor: 6 + contract: v1beta1 + - major: 0 + minor: 5 + contract: v1beta1 - major: 0 minor: 4 contract: v1beta1 diff --git a/pkg/cloud/isolated_network.go b/pkg/cloud/isolated_network.go index 2d0e1b80..27a2ad23 100644 --- a/pkg/cloud/isolated_network.go +++ b/pkg/cloud/isolated_network.go @@ -17,32 +17,40 @@ limitations under the License. package cloud import ( + "fmt" + "net" + "slices" "strconv" "strings" "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/hashicorp/go-multierror" "github.com/pkg/errors" + utilsnet "k8s.io/utils/net" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" - "sigs.k8s.io/cluster-api/util/annotations" + capcstrings "sigs.k8s.io/cluster-api-provider-cloudstack/pkg/utils/strings" + "sigs.k8s.io/cluster-api/util/record" ) type IsoNetworkIface interface { GetOrCreateIsolatedNetwork(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error + ReconcileLoadBalancer(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error - AssociatePublicIPAddress(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error - GetOrCreateLoadBalancerRule(*infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error - OpenFirewallRules(*infrav1.CloudStackIsolatedNetwork) error - GetPublicIP(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackCluster) (*cloudstack.PublicIpAddress, error) - ResolveLoadBalancerRuleDetails(*infrav1.CloudStackIsolatedNetwork) error + AssociatePublicIPAddress(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, string) (*cloudstack.PublicIpAddress, error) + CreateEgressFirewallRules(*infrav1.CloudStackIsolatedNetwork) error + GetPublicIP(*infrav1.CloudStackFailureDomain, string) (*cloudstack.PublicIpAddress, error) + GetLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork) ([]*cloudstack.LoadBalancerRule, error) + ReconcileLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster) error + GetFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) ([]*cloudstack.FirewallRule, error) + ReconcileFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster) error - AssignVMToLoadBalancerRule(isoNet *infrav1.CloudStackIsolatedNetwork, instanceID string) error + AssignVMToLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork, instanceID string) error DeleteNetwork(infrav1.Network) error DisposeIsoNetResources(*infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error } -// getOfferingID fetches an offering id. -func (c *client) getOfferingID() (string, error) { +// getNetworkOfferingID fetches the id of a network offering. +func (c *client) getNetworkOfferingID() (string, error) { offeringID, count, retErr := c.cs.NetworkOffering.GetNetworkOfferingID(NetOffering) if retErr != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(retErr) @@ -50,57 +58,47 @@ func (c *client) getOfferingID() (string, error) { } else if count != 1 { return "", errors.New("found more than one network offering") } + return offeringID, nil } -// AssociatePublicIPAddress Gets a PublicIP and associates the public IP to passed isolated network. +// AssociatePublicIPAddress gets a public IP and associates it to the isolated network. func (c *client) AssociatePublicIPAddress( fd *infrav1.CloudStackFailureDomain, isoNet *infrav1.CloudStackIsolatedNetwork, - csCluster *infrav1.CloudStackCluster, -) (retErr error) { + desiredIP string, +) (*cloudstack.PublicIpAddress, error) { // Check specified IP address is available or get an unused one if not specified. - publicAddress, err := c.GetPublicIP(fd, csCluster) + publicAddress, err := c.GetPublicIP(fd, desiredIP) if err != nil { - return errors.Wrapf(err, "fetching a public IP address") + return nil, errors.Wrap(err, "fetching a public IP address") } - isoNet.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress - if !annotations.IsExternallyManaged(csCluster) { - // Do not update the infracluster's controlPlaneEndpoint when the controlplane - // is externally managed, it is the responsibility of the externally managed - // control plane to update this. - csCluster.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress - } - isoNet.Status.PublicIPID = publicAddress.Id // Check if the address is already associated with the network. if publicAddress.Associatednetworkid == isoNet.Spec.ID { - return nil + return publicAddress, nil } // Public IP found, but not yet associated with network -- associate it. p := c.cs.Address.NewAssociateIpAddressParams() - p.SetIpaddress(isoNet.Spec.ControlPlaneEndpoint.Host) + p.SetIpaddress(publicAddress.Ipaddress) p.SetNetworkid(isoNet.Spec.ID) if _, err := c.cs.Address.AssociateIpAddress(p); err != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) - return errors.Wrapf(err, + return nil, errors.Wrapf(err, "associating public IP address with ID %s to network with ID %s", publicAddress.Id, isoNet.Spec.ID) - } else if err := c.AddClusterTag(ResourceTypeIPAddress, publicAddress.Id, csCluster); err != nil { - return errors.Wrapf(err, - "adding tag to public IP address with ID %s", publicAddress.Id) - } else if err := c.AddCreatedByCAPCTag(ResourceTypeIPAddress, isoNet.Status.PublicIPID); err != nil { - return errors.Wrapf(err, + } else if err := c.AddCreatedByCAPCTag(ResourceTypeIPAddress, publicAddress.Id); err != nil { + return nil, errors.Wrapf(err, "adding tag to public IP address with ID %s", publicAddress.Id) } - return nil + + return publicAddress, nil } // CreateIsolatedNetwork creates an isolated network in the relevant FailureDomain per passed network specification. func (c *client) CreateIsolatedNetwork(fd *infrav1.CloudStackFailureDomain, isoNet *infrav1.CloudStackIsolatedNetwork) (retErr error) { - // Get network offering ID. - offeringID, err := c.getOfferingID() + offeringID, err := c.getNetworkOfferingID() if err != nil { return err } @@ -111,17 +109,30 @@ func (c *client) CreateIsolatedNetwork(fd *infrav1.CloudStackFailureDomain, isoN if isoNet.Spec.Domain != "" { p.SetNetworkdomain(isoNet.Spec.Domain) } + if isoNet.Spec.CIDR != "" { + m, err := parseCIDR(isoNet.Spec.CIDR) + if err != nil { + return errors.Wrap(err, "parsing CIDR") + } + // Set the needed IP subnet config + p.SetGateway(m["gateway"]) + p.SetNetmask(m["netmask"]) + p.SetStartip(m["startip"]) + p.SetEndip(m["endip"]) + } resp, err := c.cs.Network.CreateNetwork(p) if err != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) return errors.Wrapf(err, "creating network with name %s", isoNet.Spec.Name) } isoNet.Spec.ID = resp.Id + isoNet.Spec.CIDR = resp.Cidr + return c.AddCreatedByCAPCTag(ResourceTypeNetwork, isoNet.Spec.ID) } -// OpenFirewallRules opens a CloudStack egress firewall for an isolated network. -func (c *client) OpenFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) (retErr error) { +// CreateEgressFirewallRules sets the egress firewall rules for an isolated network. +func (c *client) CreateEgressFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) (retErr error) { protocols := []string{NetworkProtocolTCP, NetworkProtocolUDP, NetworkProtocolICMP} for _, proto := range protocols { p := c.cs.Firewall.NewCreateEgressFirewallRuleParams(isoNet.Spec.ID, proto) @@ -145,27 +156,25 @@ func (c *client) OpenFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) (r return retErr } -// GetPublicIP gets a public IP with ID for cluster endpoint. +// GetPublicIP gets a public IP. If desiredIP is empty, it will pick the next available IP. func (c *client) GetPublicIP( fd *infrav1.CloudStackFailureDomain, - csCluster *infrav1.CloudStackCluster, + desiredIP string, ) (*cloudstack.PublicIpAddress, error) { - ip := csCluster.Spec.ControlPlaneEndpoint.Host - p := c.cs.Address.NewListPublicIpAddressesParams() p.SetAllocatedonly(false) p.SetZoneid(fd.Spec.Zone.ID) - setIfNotEmpty(ip, p.SetIpaddress) + setIfNotEmpty(desiredIP, p.SetIpaddress) publicAddresses, err := c.cs.Address.ListPublicIpAddresses(p) if err != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) return nil, err - } else if ip != "" && publicAddresses.Count == 1 { - // Endpoint specified and IP found. + } else if desiredIP != "" && publicAddresses.Count == 1 { + // Desired IP specified and IP found. // Ignore already allocated here since the IP was specified. return publicAddresses.PublicIpAddresses[0], nil } else if publicAddresses.Count > 0 { - // Endpoint not specified. Pick first available address. + // Desired IP not specified. Pick first available address. for _, v := range publicAddresses.PublicIpAddresses { if v.Allocated == "" { // Found un-allocated Public IP. return v, nil @@ -187,6 +196,7 @@ func (c *client) GetIsolatedNetwork(isoNet *infrav1.CloudStackIsolatedNetwork) ( "expected 1 Network with name %s, but got %d", isoNet.Name, count)) } else { // Got netID from the network's name. isoNet.Spec.ID = netDetails.Id + isoNet.Spec.CIDR = netDetails.Cidr return nil } @@ -197,58 +207,475 @@ func (c *client) GetIsolatedNetwork(isoNet *infrav1.CloudStackIsolatedNetwork) ( } else if count != 1 { return multierror.Append(retErr, errors.Errorf("expected 1 Network with UUID %s, but got %d", isoNet.Spec.ID, count)) } - isoNet.Name = netDetails.Name + isoNet.Spec.Name = netDetails.Name + isoNet.Spec.CIDR = netDetails.Cidr return nil } -// ResolveLoadBalancerRuleDetails resolves the details of a load balancer rule by PublicIPID and Port. -func (c *client) ResolveLoadBalancerRuleDetails( - isoNet *infrav1.CloudStackIsolatedNetwork, -) error { +// GetLoadBalancerRules fetches the current loadbalancer rules for the isolated network. +func (c *client) GetLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork) ([]*cloudstack.LoadBalancerRule, error) { p := c.cs.LoadBalancer.NewListLoadBalancerRulesParams() - p.SetPublicipid(isoNet.Status.PublicIPID) + p.SetPublicipid(isoNet.Status.APIServerLoadBalancer.IPAddressID) loadBalancerRules, err := c.cs.LoadBalancer.ListLoadBalancerRules(p) if err != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) - return errors.Wrap(err, "listing load balancer rules") + return nil, errors.Wrap(err, "listing load balancer rules") + } + + return loadBalancerRules.LoadBalancerRules, nil +} + +// ReconcileLoadBalancerRules manages the loadbalancer rules for all ports. +func (c *client) ReconcileLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster) error { + // If there is no public IP address associated with the load balancer, do nothing. + if isoNet.Status.APIServerLoadBalancer.IPAddressID == "" { + return nil + } + + lbr, err := c.GetLoadBalancerRules(isoNet) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + return errors.Wrap(err, "retrieving load balancer rules") + } + + portsAndIDs := mapExistingLoadBalancerRules(lbr) + + if csCluster.Spec.APIServerLoadBalancer.IsEnabled() { + // Load balancer enabled, reconcile the rules. + ports := gatherPorts(csCluster) + + lbRuleIDs, err := c.ensureLoadBalancerRules(isoNet, ports, portsAndIDs, csCluster) + if err != nil { + return err + } + + if err := c.cleanupObsoleteLoadBalancerRules(portsAndIDs, ports); err != nil { + return err + } + + if len(lbRuleIDs) > 1 { + capcstrings.Canonicalize(lbRuleIDs) + } + + isoNet.Status.LoadBalancerRuleIDs = lbRuleIDs + } else { + // Load balancer disabled, delete all rules. + if err := c.cleanupAllLoadBalancerRules(portsAndIDs); err != nil { + return err + } + + isoNet.Status.LoadBalancerRuleIDs = []string{} } - for _, rule := range loadBalancerRules.LoadBalancerRules { - if rule.Publicport == strconv.Itoa(int(isoNet.Spec.ControlPlaneEndpoint.Port)) { - isoNet.Status.LBRuleID = rule.Id - return nil + return nil +} + +// mapExistingLoadBalancerRules creates a lookup map for existing load balancer rules based on their public port. +func mapExistingLoadBalancerRules(lbr []*cloudstack.LoadBalancerRule) map[string]string { + portsAndIDs := make(map[string]string) + for _, rule := range lbr { + // Check if the rule is managed by CAPC. + capcManaged := false + for _, t := range rule.Tags { + if t.Key == CreatedByCAPCTagName && t.Value == "1" { + capcManaged = true + + break + } + } + if capcManaged { + portsAndIDs[rule.Publicport] = rule.Id } } - return errors.New("no load balancer rule found") + + return portsAndIDs } -// GetOrCreateLoadBalancerRule Create a load balancer rule that can be assigned to instances. -func (c *client) GetOrCreateLoadBalancerRule( - isoNet *infrav1.CloudStackIsolatedNetwork, - csCluster *infrav1.CloudStackCluster, -) (retErr error) { - // Check if rule exists. - if err := c.ResolveLoadBalancerRuleDetails(isoNet); err == nil || - !strings.Contains(strings.ToLower(err.Error()), "no load balancer rule found") { - return errors.Wrap(err, "resolving load balancer rule details") +// ensureLoadBalancerRules ensures that the necessary load balancer rules are in place. +func (c *client) ensureLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork, ports []int, portsAndIDs map[string]string, csCluster *infrav1.CloudStackCluster) ([]string, error) { + lbRuleIDs := make([]string, 0) + for _, port := range ports { + ruleID, err := c.getOrCreateLoadBalancerRule(isoNet, port, portsAndIDs) + if err != nil { + return nil, err + } + lbRuleIDs = append(lbRuleIDs, ruleID) + + // For backwards compatibility. + if port == int(csCluster.Spec.ControlPlaneEndpoint.Port) { + isoNet.Status.LBRuleID = ruleID + } + } + return lbRuleIDs, nil +} + +// getOrCreateLoadBalancerRule retrieves or creates a load balancer rule for a given port. +func (c *client) getOrCreateLoadBalancerRule(isoNet *infrav1.CloudStackIsolatedNetwork, port int, portsAndIDs map[string]string) (string, error) { + portStr := strconv.Itoa(port) + ruleID, found := portsAndIDs[portStr] + if found { + return ruleID, nil + } + // If not found, create the lb rule for port + ruleID, err := c.CreateLoadBalancerRule(isoNet, port) + if err != nil { + return "", errors.Wrap(err, "creating load balancer rule") + } + return ruleID, nil +} + +// cleanupObsoleteLoadBalancerRules deletes load balancer rules that are no longer needed. +func (c *client) cleanupObsoleteLoadBalancerRules(portsAndIDs map[string]string, ports []int) error { + for port, ruleID := range portsAndIDs { + intPort, err := strconv.Atoi(port) + if err != nil { + return errors.Wrap(err, "converting port to int") + } + if !slices.Contains(ports, intPort) { + if err := c.deleteLoadBalancerRuleByID(ruleID); err != nil { + return err + } + } + } + return nil +} + +// cleanupAllLoadBalancerRules deletes all load balancer rules created by CAPC. +func (c *client) cleanupAllLoadBalancerRules(portsAndIDs map[string]string) error { + for _, ruleID := range portsAndIDs { + if err := c.deleteLoadBalancerRuleByID(ruleID); err != nil { + return err + } + } + + return nil +} + +// deleteLoadBalancerRuleByID wraps the deletion logic with error handling. +func (c *client) deleteLoadBalancerRuleByID(ruleID string) error { + success, err := c.DeleteLoadBalancerRule(ruleID) + if err != nil { + return errors.Wrap(err, "deleting load balancer rule") } + if !success { + return errors.New("delete load balancer rule returned unsuccessful") + } + return nil +} +// CreateLoadBalancerRule configures the loadbalancer to accept traffic to a certain IP:port. +// +// Note that due to the lack of a cidrlist parameter in UpdateLoadbalancerRule, we can't use +// loadbalancer ACLs to implement the allowedCIDR functionality, and are forced to use firewall +// rules instead. See https://github.com/apache/cloudstack/issues/8382 for details. +func (c *client) CreateLoadBalancerRule(isoNet *infrav1.CloudStackIsolatedNetwork, port int) (string, error) { + name := fmt.Sprintf("K8s_API_%d", port) p := c.cs.LoadBalancer.NewCreateLoadBalancerRuleParams( - "roundrobin", "Kubernetes_API_Server", K8sDefaultAPIPort, K8sDefaultAPIPort) - p.SetPublicport(int(csCluster.Spec.ControlPlaneEndpoint.Port)) + "roundrobin", name, port, port) + p.SetPublicport(port) p.SetNetworkid(isoNet.Spec.ID) - p.SetPublicipid(isoNet.Status.PublicIPID) + p.SetPublicipid(isoNet.Status.APIServerLoadBalancer.IPAddressID) p.SetProtocol(NetworkProtocolTCP) + // Do not open the firewall to the world, we'll manage that ourselves (unfortunately). + p.SetOpenfirewall(false) resp, err := c.cs.LoadBalancer.CreateLoadBalancerRule(p) if err != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + + return "", err + } + if err := c.AddCreatedByCAPCTag(ResourceTypeLoadBalancerRule, resp.Id); err != nil { + return "", errors.Wrap(err, "adding created by CAPC tag") + } + + return resp.Id, nil +} + +// DeleteLoadBalancerRule deletes an existing load balancer rule. +func (c *client) DeleteLoadBalancerRule(id string) (bool, error) { + isCAPCManaged, err := c.IsCapcManaged(ResourceTypeLoadBalancerRule, id) + if err != nil { + return false, err + } + + if !isCAPCManaged { + return false, errors.Errorf("load balancer rule with id %s is not managed by CAPC", id) + } + + p := c.csAsync.LoadBalancer.NewDeleteLoadBalancerRuleParams(id) + resp, err := c.csAsync.LoadBalancer.DeleteLoadBalancerRule(p) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + + return false, err + } + + return resp.Success, nil +} + +// GetFirewallRules fetches the current firewall rules for the isolated network. +func (c *client) GetFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) ([]*cloudstack.FirewallRule, error) { + p := c.cs.Firewall.NewListFirewallRulesParams() + p.SetIpaddressid(isoNet.Status.APIServerLoadBalancer.IPAddressID) + p.SetNetworkid(isoNet.Spec.ID) + fwRules, err := c.cs.Firewall.ListFirewallRules(p) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + return nil, errors.Wrap(err, "listing firewall rules") + } + + return fwRules.FirewallRules, nil +} + +// ReconcileFirewallRules manages the firewall rules for all port <-> allowedCIDR combinations. +func (c *client) ReconcileFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster) error { + // If there is no public IP address associated with the load balancer, do nothing. + if isoNet.Status.APIServerLoadBalancer.IPAddressID == "" { + return nil + } + + fwr, err := c.GetFirewallRules(isoNet) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + return errors.Wrap(err, "retrieving firewall rules") + } + + portsAndIDs := mapExistingFirewallRules(fwr) + + if csCluster.Spec.APIServerLoadBalancer.IsEnabled() { + // Load balancer enabled, reconcile firewall rules. + ports := gatherPorts(csCluster) + allowedCIDRS := getCanonicalAllowedCIDRs(isoNet, csCluster) + + // A note on the implementation here: + // Due to the lack of a `cidrlist` parameter in UpdateFirewallRule, we have to manage + // firewall rules for every item in the list of allowed CIDRs. + // See https://github.com/apache/cloudstack/issues/8382 + if err := c.reconcileFirewallRulesForPorts(isoNet, fwr, ports, allowedCIDRS); err != nil { + return err + } + + if err := c.cleanupObsoleteFirewallRules(portsAndIDs, ports); err != nil { + return err + } + + // Update the list of allowed CIDRs in the status + isoNet.Status.APIServerLoadBalancer.AllowedCIDRs = allowedCIDRS + } else { + // Load balancer disabled, remove all firewall rules. + if err := c.cleanupAllFirewallRules(portsAndIDs); err != nil { + return err + } + + isoNet.Status.APIServerLoadBalancer.AllowedCIDRs = []string{} + } + + return nil +} + +// gatherPorts collects all the ports that need firewall or load balancer rules. +func gatherPorts(csCluster *infrav1.CloudStackCluster) []int { + ports := []int{int(csCluster.Spec.ControlPlaneEndpoint.Port)} + if len(csCluster.Spec.APIServerLoadBalancer.AdditionalPorts) > 0 { + ports = append(ports, csCluster.Spec.APIServerLoadBalancer.AdditionalPorts...) + } + + return ports +} + +// mapExistingFirewallRules creates a lookup map for existing firewall rules based on their port. +func mapExistingFirewallRules(fwr []*cloudstack.FirewallRule) map[int][]string { + portsAndIDs := make(map[int][]string) + for _, rule := range fwr { + // Check if the rule is managed by CAPC. + capcManaged := false + for _, t := range rule.Tags { + if t.Key == CreatedByCAPCTagName && t.Value == "1" { + capcManaged = true + + break + } + } + if capcManaged && rule.Startport == rule.Endport { + portsAndIDs[rule.Startport] = append(portsAndIDs[rule.Startport], rule.Id) + } + } + + return portsAndIDs +} + +// reconcileFirewallRulesForPorts ensures the correct firewall rules exist for the given ports and CIDRs. +func (c *client) reconcileFirewallRulesForPorts(isoNet *infrav1.CloudStackIsolatedNetwork, fwr []*cloudstack.FirewallRule, ports []int, allowedCIDRS []string) error { + for _, port := range ports { + foundCIDRs := findExistingFirewallCIDRs(fwr, port) + if err := c.deleteUnwantedFirewallRules(fwr, port, allowedCIDRS); err != nil { + return err + } + + if err := c.createMissingFirewallRules(isoNet, port, allowedCIDRS, foundCIDRs); err != nil { + return err + } + } + + return nil +} + +// findExistingFirewallCIDRs finds existing CIDRs for a specific port in the current firewall ruleset. +func findExistingFirewallCIDRs(fwr []*cloudstack.FirewallRule, port int) []string { + foundCIDRs := make([]string, 0) + for _, rule := range fwr { + if rule.Startport == port && rule.Endport == port { + foundCIDRs = append(foundCIDRs, rule.Cidrlist) + } + } + + return foundCIDRs +} + +// deleteUnwantedFirewallRules deletes firewall rules that should no longer exist. +func (c *client) deleteUnwantedFirewallRules(fwr []*cloudstack.FirewallRule, port int, allowedCIDRS []string) error { + for _, rule := range fwr { + if rule.Startport == port && rule.Endport == port && !slices.Contains(allowedCIDRS, rule.Cidrlist) { + if err := c.deleteFirewallRuleByID(rule.Id); err != nil { + return err + } + } + } + + return nil +} + +// createMissingFirewallRules creates any firewall rules that are missing. +func (c *client) createMissingFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork, port int, allowedCIDRS, foundCIDRs []string) error { + _, createCIDRs := capcstrings.SliceDiff(foundCIDRs, allowedCIDRS) + for _, cidr := range createCIDRs { + if err := c.CreateFirewallRule(isoNet, port, cidr); err != nil { + return errors.Wrap(err, "creating firewall rule") + } + } + + return nil +} + +// cleanupObsoleteFirewallRules deletes firewall rules that are no longer needed. +func (c *client) cleanupObsoleteFirewallRules(portsAndIDs map[int][]string, ports []int) error { + for port, ruleIDs := range portsAndIDs { + if !slices.Contains(ports, port) { + for _, ruleID := range ruleIDs { + if err := c.deleteFirewallRuleByID(ruleID); err != nil { + return err + } + } + } + } + + return nil +} + +// cleanupAllFirewallRules deletes all firewall rules created by CAPC. +func (c *client) cleanupAllFirewallRules(portsAndIDs map[int][]string) error { + for _, ruleIDs := range portsAndIDs { + for _, ruleID := range ruleIDs { + if err := c.deleteFirewallRuleByID(ruleID); err != nil { + return err + } + } + } + + return nil +} + +// deleteFirewallRuleByID wraps the firewall rule deletion logic with error handling. +func (c *client) deleteFirewallRuleByID(ruleID string) error { + success, err := c.DeleteFirewallRule(ruleID) + if err != nil { + return errors.Wrap(err, "deleting firewall rule") + } + if !success { + return errors.New("delete firewall rule returned unsuccessful") + } + + return nil +} + +// CreateFirewallRule creates a firewall rule to allow traffic from a certain CIDR to a port on our public IP. +func (c *client) CreateFirewallRule(isoNet *infrav1.CloudStackIsolatedNetwork, port int, cidr string) error { + cidrList := []string{cidr} + p := c.cs.Firewall.NewCreateFirewallRuleParams(isoNet.Status.APIServerLoadBalancer.IPAddressID, NetworkProtocolTCP) + p.SetStartport(port) + p.SetEndport(port) + p.SetCidrlist(cidrList) + resp, err := c.cs.Firewall.CreateFirewallRule(p) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + return err } - isoNet.Status.LBRuleID = resp.Id + if err := c.AddCreatedByCAPCTag(ResourceTypeFirewallRule, resp.Id); err != nil { + return errors.Wrap(err, "adding created by CAPC tag") + } + return nil } +// DeleteFirewallRule deletes a firewall rule. +func (c *client) DeleteFirewallRule(id string) (bool, error) { + isCAPCManaged, err := c.IsCapcManaged(ResourceTypeFirewallRule, id) + if err != nil { + return false, err + } + + if !isCAPCManaged { + return false, errors.Errorf("firewall rule with id %s is not managed by CAPC", id) + } + + p := c.csAsync.Firewall.NewDeleteFirewallRuleParams(id) + resp, err := c.csAsync.Firewall.DeleteFirewallRule(p) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + + return false, err + } + + return resp.Success, nil +} + +// getCanonicalAllowedCIDRs gets a filtered list of CIDRs which should be allowed to access the API server loadbalancer. +// Invalid CIDRs are filtered from the list and emil a warning event. +// It returns a canonical representation that can be directly compared with other canonicalized lists. +func getCanonicalAllowedCIDRs(isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster) []string { + allowedCIDRs := []string{} + + if csCluster.Spec.APIServerLoadBalancer != nil && len(csCluster.Spec.APIServerLoadBalancer.AllowedCIDRs) > 0 { + allowedCIDRs = append(allowedCIDRs, csCluster.Spec.APIServerLoadBalancer.AllowedCIDRs...) + + // Add our own outgoing IP + if len(isoNet.Status.PublicIPAddress) > 0 { + allowedCIDRs = append(allowedCIDRs, isoNet.Status.PublicIPAddress) + } + } else { + // If there are no specific CIDRs defined to allow traffic from, default to allow all. + allowedCIDRs = append(allowedCIDRs, "0.0.0.0/0") + } + + // Filter invalid CIDRs and convert any IPs into CIDRs. + validCIDRs := []string{} + for _, v := range allowedCIDRs { + switch { + case utilsnet.IsIPv4String(v): + validCIDRs = append(validCIDRs, v+"/32") + case utilsnet.IsIPv4CIDRString(v): + validCIDRs = append(validCIDRs, v) + default: + record.Warnf(csCluster, "FailedIPAddressValidation", "%s is not a valid IPv4 nor CIDR address and will not get applied to firewall rules", v) + } + } + + // Canonicalize by sorting and removing duplicates. + return capcstrings.Canonicalize(validCIDRs) +} + // GetOrCreateIsolatedNetwork fetches or builds out the necessary structures for isolated network use. func (c *client) GetOrCreateIsolatedNetwork( fd *infrav1.CloudStackFailureDomain, @@ -256,70 +683,110 @@ func (c *client) GetOrCreateIsolatedNetwork( csCluster *infrav1.CloudStackCluster, ) error { // Get or create the isolated network itself and resolve details into passed custom resources. - net := isoNet.Network() - if err := c.ResolveNetwork(net); err != nil { // Doesn't exist, create isolated network. + network := isoNet.Network() + if err := c.ResolveNetwork(network); err != nil { // Doesn't exist, create isolated network. if err = c.CreateIsolatedNetwork(fd, isoNet); err != nil { return errors.Wrap(err, "creating a new isolated network") } - } else { // Network existed and was resolved. Set ID on isoNet CloudStackIsolatedNetwork in case it only had name set. - isoNet.Spec.ID = net.ID + } else { + // Network existed and was resolved. Set ID on isoNet CloudStackIsolatedNetwork in case it only had name set. + isoNet.Spec.ID = network.ID + isoNet.Spec.CIDR = network.CIDR } - // Tag the created network. - networkID := isoNet.Spec.ID - if err := c.AddClusterTag(ResourceTypeNetwork, networkID, csCluster); err != nil { - return errors.Wrapf(err, "tagging network with id %s", networkID) + // Open the Isolated Network egress firewall. + return errors.Wrap(c.CreateEgressFirewallRules(isoNet), "opening the isolated network's egress firewall") +} + +// ReconcileLoadBalancer configures the API server load balancer. +func (c *client) ReconcileLoadBalancer( + fd *infrav1.CloudStackFailureDomain, + isoNet *infrav1.CloudStackIsolatedNetwork, + csCluster *infrav1.CloudStackCluster, +) error { + // Check/set ControlPlaneEndpoint port. + // Prefer csCluster ControlPlaneEndpoint port. Use isonet port if CP missing. Set to default if both missing. + if csCluster.Spec.ControlPlaneEndpoint.Port != 0 { + isoNet.Spec.ControlPlaneEndpoint.Port = csCluster.Spec.ControlPlaneEndpoint.Port + } else if isoNet.Spec.ControlPlaneEndpoint.Port != 0 { // Override default public port if endpoint port specified. + csCluster.Spec.ControlPlaneEndpoint.Port = isoNet.Spec.ControlPlaneEndpoint.Port + } else { + csCluster.Spec.ControlPlaneEndpoint.Port = 6443 + isoNet.Spec.ControlPlaneEndpoint.Port = 6443 } - if !annotations.IsExternallyManaged(csCluster) { + // Associate public IP with the load balancer if enabled. + /* TODO: implement possibility for load balancer to use a different IP than isonet + if csCluster.Spec.APIServerLoadBalancer.IsEnabled() { // Associate Public IP with CloudStackIsolatedNetwork if err := c.AssociatePublicIPAddress(fd, isoNet, csCluster); err != nil { return errors.Wrapf(err, "associating public IP address to csCluster") } + }*/ - // Check/set ControlPlaneEndpoint port. - // Prefer csCluster ControlPlaneEndpoint port. Use isonet port if CP missing. Set to default if both missing. - if csCluster.Spec.ControlPlaneEndpoint.Port != 0 { - isoNet.Spec.ControlPlaneEndpoint.Port = csCluster.Spec.ControlPlaneEndpoint.Port - } else if isoNet.Spec.ControlPlaneEndpoint.Port != 0 { // Override default public port if endpoint port specified. - csCluster.Spec.ControlPlaneEndpoint.Port = isoNet.Spec.ControlPlaneEndpoint.Port - } else { - csCluster.Spec.ControlPlaneEndpoint.Port = 6443 - isoNet.Spec.ControlPlaneEndpoint.Port = 6443 - } + // Set up load balancing rules to map VM ports to Public IP ports. + if err := c.ReconcileLoadBalancerRules(isoNet, csCluster); err != nil { + return errors.Wrap(err, "reconciling load balancing rules") + } + + // Set up firewall rules to manage access to load balancer public IP ports. + if err := c.ReconcileFirewallRules(isoNet, csCluster); err != nil { + return errors.Wrap(err, "reconciling firewall rules") + } - // Setup a load balancing rule to map VMs to Public IP. - if err := c.GetOrCreateLoadBalancerRule(isoNet, csCluster); err != nil { - return errors.Wrap(err, "getting or creating load balancing rule") + if !csCluster.Spec.APIServerLoadBalancer.IsEnabled() && isoNet.Status.APIServerLoadBalancer != nil { + // If the APIServerLoadBalancer has been disabled, release its IP unless it's the SNAT IP. + released, err := c.DisassociatePublicIPAddressIfNotInUse(isoNet.Status.APIServerLoadBalancer.IPAddressID) + if err != nil { + return errors.Wrap(err, "disassociating public IP address") } + if released { + isoNet.Status.APIServerLoadBalancer.IPAddress = "" + isoNet.Status.APIServerLoadBalancer.IPAddressID = "" + } + + // Clear the load balancer status as it is disabled. + isoNet.Status.APIServerLoadBalancer = nil } - // Open the Isolated Network on endopint port. - return errors.Wrap(c.OpenFirewallRules(isoNet), "opening the isolated network's firewall") + return nil } -// AssignVMToLoadBalancerRule assigns a VM instance to a load balancing rule (specifying lb membership). -func (c *client) AssignVMToLoadBalancerRule(isoNet *infrav1.CloudStackIsolatedNetwork, instanceID string) (retErr error) { +// AssignVMToLoadBalancerRules assigns a VM to the load balancing rules listed in isoNet.Status.LoadBalancerRuleIDs, +// if not already assigned. +func (c *client) AssignVMToLoadBalancerRules(isoNet *infrav1.CloudStackIsolatedNetwork, instanceID string) error { + var found bool + for _, lbRuleID := range isoNet.Status.LoadBalancerRuleIDs { + // Check that the instance isn't already in LB rotation. + found = false + lbRuleInstances, err := c.cs.LoadBalancer.ListLoadBalancerRuleInstances( + c.cs.LoadBalancer.NewListLoadBalancerRuleInstancesParams(lbRuleID)) + if err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) - // Check that the instance isn't already in LB rotation. - lbRuleInstances, retErr := c.cs.LoadBalancer.ListLoadBalancerRuleInstances( - c.cs.LoadBalancer.NewListLoadBalancerRuleInstancesParams(isoNet.Status.LBRuleID)) - if retErr != nil { - c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(retErr) - return retErr - } - for _, instance := range lbRuleInstances.LoadBalancerRuleInstances { - if instance.Id == instanceID { // Already assigned to load balancer.. - return nil + return err + } + for _, instance := range lbRuleInstances.LoadBalancerRuleInstances { + if instance.Id == instanceID { // Already assigned to load balancer. + found = true + + break + } + } + + if !found { + // Assign to Load Balancer. + p := c.cs.LoadBalancer.NewAssignToLoadBalancerRuleParams(lbRuleID) + p.SetVirtualmachineids([]string{instanceID}) + if _, err = c.cs.LoadBalancer.AssignToLoadBalancerRule(p); err != nil { + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + + return err + } } } - // Assign to Load Balancer. - p := c.cs.LoadBalancer.NewAssignToLoadBalancerRuleParams(isoNet.Status.LBRuleID) - p.SetVirtualmachineids([]string{instanceID}) - _, retErr = c.cs.LoadBalancer.AssignToLoadBalancerRule(p) - c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(retErr) - return retErr + return nil } // DeleteNetwork deletes an isolated network. @@ -333,15 +800,29 @@ func (c *client) DeleteNetwork(net infrav1.Network) error { func (c *client) DisposeIsoNetResources( isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster, -) (retError error) { +) error { + // Release the load balancer IP, if the load balancer is enabled and its IP is different from the isonet public IP. + if csCluster.Spec.APIServerLoadBalancer.IsEnabled() && isoNet.Status.APIServerLoadBalancer.IPAddressID != "" && + isoNet.Status.APIServerLoadBalancer.IPAddressID != isoNet.Status.PublicIPID { + if err := c.DeleteClusterTag(ResourceTypeIPAddress, isoNet.Status.APIServerLoadBalancer.IPAddressID, csCluster); err != nil { + return err + } + if _, err := c.DisassociatePublicIPAddressIfNotInUse(isoNet.Status.APIServerLoadBalancer.IPAddressID); err != nil { + return err + } + } + + // Release the isolated network public IP. if isoNet.Status.PublicIPID != "" { if err := c.DeleteClusterTag(ResourceTypeIPAddress, isoNet.Status.PublicIPID, csCluster); err != nil { return err } - if err := c.DisassociatePublicIPAddressIfNotInUse(isoNet); err != nil { + if _, err := c.DisassociatePublicIPAddressIfNotInUse(isoNet.Status.PublicIPID); err != nil { return err } } + + // Remove this cluster's tag from the isolated network. if err := c.RemoveClusterTagFromNetwork(csCluster, *isoNet.Network()); err != nil { return err } @@ -370,32 +851,67 @@ func (c *client) DeleteNetworkIfNotInUse(net infrav1.Network) (retError error) { return nil } -// DisassociatePublicIPAddressIfNotInUse removes a CloudStack public IP association from passed isolated network -// if it is no longer in use (indicated by in use tags). -func (c *client) DisassociatePublicIPAddressIfNotInUse(isoNet *infrav1.CloudStackIsolatedNetwork) (retError error) { - if tagsAllowDisposal, err := c.DoClusterTagsAllowDisposal(ResourceTypeIPAddress, isoNet.Status.PublicIPID); err != nil { - return err - } else if publicIP, _, err := c.cs.Address.GetPublicIpAddressByID(isoNet.Status.PublicIPID); err != nil { +// DisassociatePublicIPAddressIfNotInUse removes a CloudStack public IP association from an isolated network +// if it is no longer in use (indicated by in use tags). It returns a bool indicating whether an IP was actually +// disassociated, and an error in case an error occurred. +func (c *client) DisassociatePublicIPAddressIfNotInUse(ipAddressID string) (bool, error) { + if ipAddressID == "" { + return false, errors.New("ipAddressID cannot be empty") + } + if tagsAllowDisposal, err := c.DoClusterTagsAllowDisposal(ResourceTypeIPAddress, ipAddressID); err != nil { + return false, err + } else if publicIP, _, err := c.cs.Address.GetPublicIpAddressByID(ipAddressID); err != nil { c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) - return err + return false, err } else if publicIP == nil || publicIP.Issourcenat { // Can't disassociate an address if it's the source NAT address. - return nil + return false, nil } else if tagsAllowDisposal { - return c.DisassociatePublicIPAddress(isoNet) + if err := c.DisassociatePublicIPAddress(ipAddressID); err != nil { + return false, err + } + + return true, nil } - return nil + + return false, nil } -// DisassociatePublicIPAddress removes a CloudStack public IP association from passed isolated network. -func (c *client) DisassociatePublicIPAddress(isoNet *infrav1.CloudStackIsolatedNetwork) (retErr error) { +// DisassociatePublicIPAddress removes a CloudStack public IP association an isolated network. +func (c *client) DisassociatePublicIPAddress(ipAddressID string) error { + if ipAddressID == "" { + return errors.New("ipAddressID cannot be empty") + } + // Remove the CAPC creation tag, so it won't be there the next time this address is associated. - retErr = c.DeleteCreatedByCAPCTag(ResourceTypeIPAddress, isoNet.Status.PublicIPID) - if retErr != nil { - return retErr + err := c.DeleteCreatedByCAPCTag(ResourceTypeIPAddress, ipAddressID) + if err != nil { + return err } - p := c.cs.Address.NewDisassociateIpAddressParams(isoNet.Status.PublicIPID) - _, retErr = c.cs.Address.DisassociateIpAddress(p) - c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(retErr) - return retErr + p := c.cs.Address.NewDisassociateIpAddressParams(ipAddressID) + _, err = c.cs.Address.DisassociateIpAddress(p) + c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err) + + return err +} + +// parseCIDR parses a CIDR-formatted string into the components required for CreateNetwork. +func parseCIDR(cidr string) (map[string]string, error) { + m := make(map[string]string, 4) + + ip, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + return nil, fmt.Errorf("unable to parse cidr %s: %s", cidr, err) + } + + msk := ipnet.Mask + sub := ip.Mask(msk) + + m["netmask"] = fmt.Sprintf("%d.%d.%d.%d", msk[0], msk[1], msk[2], msk[3]) + m["gateway"] = fmt.Sprintf("%d.%d.%d.%d", sub[0], sub[1], sub[2], sub[3]+1) + m["startip"] = fmt.Sprintf("%d.%d.%d.%d", sub[0], sub[1], sub[2], sub[3]+2) + m["endip"] = fmt.Sprintf("%d.%d.%d.%d", + sub[0]+(0xff-msk[0]), sub[1]+(0xff-msk[1]), sub[2]+(0xff-msk[2]), sub[3]+(0xff-msk[3]-1)) + + return m, nil } diff --git a/pkg/cloud/isolated_network_test.go b/pkg/cloud/isolated_network_test.go index 77d77f95..7846c45c 100644 --- a/pkg/cloud/isolated_network_test.go +++ b/pkg/cloud/isolated_network_test.go @@ -17,6 +17,7 @@ limitations under the License. package cloud_test import ( + "k8s.io/utils/pointer" "strconv" csapi "github.com/apache/cloudstack-go/v2/cloudstack" @@ -77,13 +78,7 @@ var _ = Describe("Network", func() { ns.EXPECT().GetNetworkByName(dummies.ISONet1.Name).Return(nil, 0, nil) ns.EXPECT().GetNetworkByID(dummies.ISONet1.ID).Return(nil, 0, nil) ns.EXPECT().CreateNetwork(gomock.Any()).Return(&csapi.CreateNetworkResponse{Id: dummies.ISONet1.ID}, nil) - as.EXPECT().NewListPublicIpAddressesParams().Return(&csapi.ListPublicIpAddressesParams{}) - as.EXPECT().ListPublicIpAddresses(gomock.Any()). - Return(&csapi.ListPublicIpAddressesResponse{ - Count: 1, - PublicIpAddresses: []*csapi.PublicIpAddress{{Id: dummies.PublicIPID, Ipaddress: "fakeIP"}}}, nil) - as.EXPECT().NewAssociateIpAddressParams().Return(&csapi.AssociateIpAddressParams{}) - as.EXPECT().AssociateIpAddress(gomock.Any()) + fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()). DoAndReturn(func(networkid string, protocol string) *csapi.CreateEgressFirewallRuleParams { p := &csapi.CreateEgressFirewallRuleParams{} @@ -103,25 +98,41 @@ var _ = Describe("Network", func() { fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP). Return(&csapi.CreateEgressFirewallRuleResponse{}, nil)) - // Will add cluster tag once to Network and once to PublicIP. - createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}} - gomock.InOrder( - rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}), - rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil), - rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}), - rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil)) - - // Will add creation and cluster tags to network and PublicIP. + // Will add creation tags to network. rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&csapi.CreateTagsParams{}).Times(4) - rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(4) + Return(&csapi.CreateTagsParams{}) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil) - lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) - lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( - &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ - {Publicport: strconv.Itoa(int(dummies.EndPointPort)), Id: dummies.LBRuleID}}}, nil) + Ω(client.GetOrCreateIsolatedNetwork(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Spec.ID).ShouldNot(BeEmpty()) + }) + + It("resolves the existing isolated network", func() { + dummies.SetClusterSpecToNet(&dummies.ISONet1) + + ns.EXPECT().GetNetworkByName(dummies.ISONet1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.ISONet1), 1, nil) + + fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()). + DoAndReturn(func(networkid string, protocol string) *csapi.CreateEgressFirewallRuleParams { + p := &csapi.CreateEgressFirewallRuleParams{} + if protocol == "icmp" { + p.SetIcmptype(-1) + p.SetIcmpcode(-1) + } + return p + }).Times(3) + + ruleParamsICMP := &csapi.CreateEgressFirewallRuleParams{} + ruleParamsICMP.SetIcmptype(-1) + ruleParamsICMP.SetIcmpcode(-1) + gomock.InOrder( + fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}). + Return(&csapi.CreateEgressFirewallRuleResponse{}, nil).Times(2), + fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP). + Return(&csapi.CreateEgressFirewallRuleResponse{}, nil)) Ω(client.GetOrCreateIsolatedNetwork(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Spec.ID).ShouldNot(BeEmpty()) }) It("fails to get network offering from CloudStack", func() { @@ -135,8 +146,8 @@ var _ = Describe("Network", func() { }) }) - Context("for a closed firewall", func() { - It("OpenFirewallRule asks CloudStack to open the firewall", func() { + Context("for a closed egress firewall", func() { + It("CreateEgressFirewallRules asks CloudStack to open the egress firewall", func() { dummies.Zone1.Network = dummies.ISONet1 fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()). DoAndReturn(func(networkid string, protocol string) *csapi.CreateEgressFirewallRuleParams { @@ -157,12 +168,12 @@ var _ = Describe("Network", func() { fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP). Return(&csapi.CreateEgressFirewallRuleResponse{}, nil)) - Ω(client.OpenFirewallRules(dummies.CSISONet1)).Should(Succeed()) + Ω(client.CreateEgressFirewallRules(dummies.CSISONet1)).Should(Succeed()) }) }) - Context("for an open firewall", func() { - It("OpenFirewallRule asks CloudStack to open the firewall anyway, but doesn't fail", func() { + Context("for an open egress firewall", func() { + It("CreateEgressFirewallRules asks CloudStack to open the firewall anyway, but doesn't fail", func() { dummies.Zone1.Network = dummies.ISONet1 fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()). @@ -184,19 +195,19 @@ var _ = Describe("Network", func() { fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP). Return(&csapi.CreateEgressFirewallRuleResponse{}, nil)) - Ω(client.OpenFirewallRules(dummies.CSISONet1)).Should(Succeed()) + Ω(client.CreateEgressFirewallRules(dummies.CSISONet1)).Should(Succeed()) }) }) Context("in an isolated network with public IPs available", func() { - It("will resolve public IP details given an endpoint spec", func() { + It("will resolve public IP details given an endpoint host", func() { as.EXPECT().NewListPublicIpAddressesParams().Return(&csapi.ListPublicIpAddressesParams{}) as.EXPECT().ListPublicIpAddresses(gomock.Any()). Return(&csapi.ListPublicIpAddressesResponse{ Count: 1, PublicIpAddresses: []*csapi.PublicIpAddress{{Id: "PublicIPID", Ipaddress: ipAddress}}, }, nil) - publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster) + publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster.Spec.ControlPlaneEndpoint.Host) Ω(err).Should(Succeed()) Ω(publicIPAddress).ShouldNot(BeNil()) Ω(publicIPAddress.Ipaddress).Should(Equal(ipAddress)) @@ -211,7 +222,7 @@ var _ = Describe("Network", func() { Count: 0, PublicIpAddresses: []*csapi.PublicIpAddress{}, }, nil) - publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster) + publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster.Spec.ControlPlaneEndpoint.Host) Ω(publicIPAddress).Should(BeNil()) Ω(err.Error()).Should(ContainSubstring("no public addresses found in available networks")) }) @@ -232,7 +243,7 @@ var _ = Describe("Network", func() { Associatednetworkid: "1", }}, }, nil) - publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster) + publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster.Spec.ControlPlaneEndpoint.Host) Ω(publicIPAddress).Should(BeNil()) Ω(err.Error()).Should(ContainSubstring("all Public IP Address(es) found were already allocated")) }) @@ -249,18 +260,14 @@ var _ = Describe("Network", func() { aip := &csapi.AssociateIpAddressParams{} as.EXPECT().NewAssociateIpAddressParams().Return(aip) as.EXPECT().AssociateIpAddress(aip).Return(&csapi.AssociateIpAddressResponse{}, nil) - // Will add cluster tag once to Network and once to PublicIP. - createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}} - gomock.InOrder( - rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}), - rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil)) // Will add creation and cluster tags to network and PublicIP. rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&csapi.CreateTagsParams{}).Times(2) - rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(2) + Return(&csapi.CreateTagsParams{}) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil) - Ω(client.AssociatePublicIPAddress(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + _, err := client.AssociatePublicIPAddress(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster.Spec.ControlPlaneEndpoint.Host) + Ω(err).Should(Succeed()) }) It("Failure Associating Public IP to Isolated network", func() { @@ -273,31 +280,152 @@ var _ = Describe("Network", func() { aip := &csapi.AssociateIpAddressParams{} as.EXPECT().NewAssociateIpAddressParams().Return(aip) as.EXPECT().AssociateIpAddress(aip).Return(nil, errors.New("Failed to allocate IP address")) - Ω(client.AssociatePublicIPAddress(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster).Error()).Should(ContainSubstring("associating public IP address with ID")) + + _, err := client.AssociatePublicIPAddress(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster.Spec.ControlPlaneEndpoint.Host) + Ω(err.Error()).Should(ContainSubstring("associating public IP address with ID")) }) }) - Context("The specific load balancer rule does exist", func() { + Context("With an enabled API load balancer", func() { + It("reconciles the required load balancer and firewall rules", func() { + dummies.SetClusterSpecToNet(&dummies.ISONet1) + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + + /* + as.EXPECT().NewListPublicIpAddressesParams().Return(&csapi.ListPublicIpAddressesParams{}) + as.EXPECT().ListPublicIpAddresses(gomock.Any()). + Return(&csapi.ListPublicIpAddressesResponse{ + Count: 1, + PublicIpAddresses: []*csapi.PublicIpAddress{{Id: dummies.LoadBalancerIPID, Ipaddress: "fakeLBIP"}}}, nil) + as.EXPECT().NewAssociateIpAddressParams().Return(&csapi.AssociateIpAddressParams{}) + as.EXPECT().AssociateIpAddress(gomock.Any())*/ + + lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) + lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( + &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Id: dummies.FWRuleID, + Cidrlist: "0.0.0.0/0", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + Ω(client.ReconcileLoadBalancer(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID).Should(Equal(dummies.LoadBalancerIPID)) + }) + }) + + Context("With a disabled API load balancer", func() { + It("deletes existing load balancer and firewall rules, disassociates IP", func() { + dummies.SetClusterSpecToNet(&dummies.ISONet1) + dummies.CSCluster.Spec.APIServerLoadBalancer = nil + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + + createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}} + rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}).Times(3) + rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil).Times(3) + + lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) + lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( + &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Id: dummies.FWRuleID, + Cidrlist: "0.0.0.0/0", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + lbs.EXPECT().NewDeleteLoadBalancerRuleParams(gomock.Any()). + Return(&csapi.DeleteLoadBalancerRuleParams{}).Times(1) + lbs.EXPECT().DeleteLoadBalancerRule(gomock.Any()). + Return(&csapi.DeleteLoadBalancerRuleResponse{Success: true}, nil).Times(1) + + fs.EXPECT().NewDeleteFirewallRuleParams(dummies.FWRuleID).DoAndReturn(func(ruleid string) *csapi.DeleteFirewallRuleParams { + p := &csapi.DeleteFirewallRuleParams{} + p.SetId(ruleid) + return p + }) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Return(&csapi.DeleteFirewallRuleResponse{Success: true}, nil).Times(1) + + as.EXPECT().GetPublicIpAddressByID(dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID).Return(&csapi.PublicIpAddress{}, 1, nil) + + rtdp := &csapi.DeleteTagsParams{} + rs.EXPECT().NewDeleteTagsParams(gomock.Any(), gomock.Any()).Return(rtdp) + rs.EXPECT().DeleteTags(rtdp).Return(&csapi.DeleteTagsResponse{}, nil) + as.EXPECT().NewDisassociateIpAddressParams(dummies.LoadBalancerIPID).Return(&csapi.DisassociateIpAddressParams{}) + as.EXPECT().DisassociateIpAddress(gomock.Any()).Return(&csapi.DisassociateIpAddressResponse{}, nil) + + Ω(client.ReconcileLoadBalancer(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Status.APIServerLoadBalancer).Should(BeNil()) + }) + }) + + Context("The specific load balancer rule exists", func() { It("resolves the rule's ID", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ - {Publicport: strconv.Itoa(int(dummies.EndPointPort)), Id: dummies.LBRuleID}}}, nil) + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) - dummies.CSISONet1.Status.LBRuleID = "" - Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSISONet1)).Should(Succeed()) - Ω(dummies.CSISONet1.Status.LBRuleID).Should(Equal(dummies.LBRuleID)) + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{} + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal(dummies.LoadBalancerRuleIDs)) }) - It("Failed to resolve LB rule details", func() { + It("when API load balancer additional ports are defined, resolves all rule IDs", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts, 456) lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ - {Publicport: "differentPublicPort", Id: dummies.LBRuleID}}}, nil) + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + { + Id: "FakeLBRuleID2", + Publicport: strconv.Itoa(456), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) - dummies.CSISONet1.Status.LBRuleID = "" - Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSISONet1).Error()). - Should(Equal("no load balancer rule found")) + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{} + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + dummies.LoadBalancerRuleIDs = []string{dummies.LBRuleID, "FakeLBRuleID2"} + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal(dummies.LoadBalancerRuleIDs)) }) It("Failed to list LB rules", func() { @@ -305,54 +433,82 @@ var _ = Describe("Network", func() { lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( nil, fakeError) - dummies.CSISONet1.Status.LBRuleID = "" - Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSISONet1).Error()). - Should(ContainSubstring("listing load balancer rules")) + Ω(client.GetLoadBalancerRules(dummies.CSISONet1)).Error().Should(MatchError(ContainSubstring("listing load balancer rules"))) }) It("doesn't create a new load balancer rule on create", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()). Return(&csapi.ListLoadBalancerRulesResponse{ LoadBalancerRules: []*csapi.LoadBalancerRule{ - {Publicport: strconv.Itoa(int(dummies.EndPointPort)), Id: dummies.LBRuleID}}}, nil) + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + }, + }, nil) - Ω(client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) - Ω(dummies.CSISONet1.Status.LBRuleID).Should(Equal(dummies.LBRuleID)) + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal(dummies.LoadBalancerRuleIDs)) }) }) Context("Assign VM to Load Balancer rule", func() { It("Associates VM to LB rule", func() { - dummies.CSISONet1.Status.LBRuleID = "lbruleid" + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{"lbruleid"} lbip := &csapi.ListLoadBalancerRuleInstancesParams{} albp := &csapi.AssignToLoadBalancerRuleParams{} - lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LBRuleID). + lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]). Return(lbip) lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{}, nil) - lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LBRuleID).Return(albp) + lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]).Return(albp) lbs.EXPECT().AssignToLoadBalancerRule(albp).Return(&csapi.AssignToLoadBalancerRuleResponse{}, nil) - Ω(client.AssignVMToLoadBalancerRule(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed()) + Ω(client.AssignVMToLoadBalancerRules(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed()) + }) + + It("With additionalPorts defined, associates VM to all related LB rules", func() { + dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts, 456) + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{dummies.LBRuleID, "FakeLBRuleID2"} + lbip := &csapi.ListLoadBalancerRuleInstancesParams{} + albp := &csapi.AssignToLoadBalancerRuleParams{} + gomock.InOrder( + lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]). + Return(lbip), + lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{}, nil), + lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]).Return(albp), + lbs.EXPECT().AssignToLoadBalancerRule(albp).Return(&csapi.AssignToLoadBalancerRuleResponse{}, nil), + + lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[1]). + Return(lbip), + lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{}, nil), + lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[1]).Return(albp), + lbs.EXPECT().AssignToLoadBalancerRule(albp).Return(&csapi.AssignToLoadBalancerRuleResponse{}, nil), + ) + + Ω(client.AssignVMToLoadBalancerRules(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed()) }) It("Associating VM to LB rule fails", func() { - dummies.CSISONet1.Status.LBRuleID = "lbruleid" + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{"lbruleid"} lbip := &csapi.ListLoadBalancerRuleInstancesParams{} albp := &csapi.AssignToLoadBalancerRuleParams{} - lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LBRuleID). + lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]). Return(lbip) lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{}, nil) - lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LBRuleID).Return(albp) + lbs.EXPECT().NewAssignToLoadBalancerRuleParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]).Return(albp) lbs.EXPECT().AssignToLoadBalancerRule(albp).Return(nil, fakeError) - Ω(client.AssignVMToLoadBalancerRule(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).ShouldNot(Succeed()) + Ω(client.AssignVMToLoadBalancerRules(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).ShouldNot(Succeed()) }) It("LB Rule already assigned to VM", func() { - dummies.CSISONet1.Status.LBRuleID = "lbruleid" + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{"lbruleid"} lbip := &csapi.ListLoadBalancerRuleInstancesParams{} - lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LBRuleID). + lbs.EXPECT().NewListLoadBalancerRuleInstancesParams(dummies.CSISONet1.Status.LoadBalancerRuleIDs[0]). Return(lbip) lbs.EXPECT().ListLoadBalancerRuleInstances(lbip).Return(&csapi.ListLoadBalancerRuleInstancesResponse{ Count: 1, @@ -361,35 +517,114 @@ var _ = Describe("Network", func() { }}, }, nil) - Ω(client.AssignVMToLoadBalancerRule(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed()) + Ω(client.AssignVMToLoadBalancerRules(dummies.CSISONet1, *dummies.CSMachine1.Spec.InstanceID)).Should(Succeed()) }) }) Context("load balancer rule does not exist", func() { - It("calls cloudstack to create a new load balancer rule.", func() { + It("calls CloudStack to create a new load balancer rule", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()). Return(&csapi.ListLoadBalancerRulesResponse{ - LoadBalancerRules: []*csapi.LoadBalancerRule{{Publicport: "7443", Id: dummies.LBRuleID}}}, nil) + LoadBalancerRules: []*csapi.LoadBalancerRule{}}, nil) + lbs.EXPECT().NewCreateLoadBalancerRuleParams(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateLoadBalancerRuleParams{}) + lbs.EXPECT().CreateLoadBalancerRule(gomock.Any()). + Return(&csapi.CreateLoadBalancerRuleResponse{Id: dummies.LBRuleID}, nil) + lbs.EXPECT().NewDeleteLoadBalancerRuleParams(gomock.Any()).Times(0) + lbs.EXPECT().DeleteLoadBalancerRule(gomock.Any()).Times(0) + rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateTagsParams{}).Times(1) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(1) + + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + loadBalancerRuleIDs := []string{dummies.LBRuleID} + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal(loadBalancerRuleIDs)) + }) + + It("when API load balancer additional ports are defined, creates additional rules", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts, 456) + lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) + lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( + &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + lbs.EXPECT().NewCreateLoadBalancerRuleParams(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&csapi.CreateLoadBalancerRuleParams{}) lbs.EXPECT().CreateLoadBalancerRule(gomock.Any()). Return(&csapi.CreateLoadBalancerRuleResponse{Id: "2ndLBRuleID"}, nil) + lbs.EXPECT().NewDeleteLoadBalancerRuleParams(gomock.Any()).Times(0) + lbs.EXPECT().DeleteLoadBalancerRule(gomock.Any()).Times(0) + rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateTagsParams{}).Times(1) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(1) - Ω(client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) - Ω(dummies.CSISONet1.Status.LBRuleID).Should(Equal("2ndLBRuleID")) + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{dummies.LBRuleID} + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + dummies.LoadBalancerRuleIDs = []string{"2ndLBRuleID", dummies.LBRuleID} + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal(dummies.LoadBalancerRuleIDs)) + }) + + It("when API load balancer additional ports are defined, and a port is removed, deletes related rules", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) + lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( + &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + { + Id: "2ndLBRuleID", + Publicport: strconv.Itoa(456), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + lbs.EXPECT().NewDeleteLoadBalancerRuleParams(gomock.Any()). + Return(&csapi.DeleteLoadBalancerRuleParams{}).Times(1) + lbs.EXPECT().DeleteLoadBalancerRule(gomock.Any()). + Return(&csapi.DeleteLoadBalancerRuleResponse{Success: true}, nil).Times(1) + rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}).Times(1) + rs.EXPECT().ListTags(gomock.Any()).Return(&csapi.ListTagsResponse{ + Count: 1, + Tags: []*csapi.Tag{{ + Key: cloud.CreatedByCAPCTagName, + Value: "1", + }}, + }, nil).Times(1) + + dummies.CSISONet1.Status.LoadBalancerRuleIDs = []string{"2ndLBRuleID", dummies.LBRuleID} + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + dummies.LoadBalancerRuleIDs = []string{dummies.LBRuleID} + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal(dummies.LoadBalancerRuleIDs)) }) It("Fails to resolve load balancer rule details", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()). Return(nil, fakeError) - err := client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster) + err := client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster) Ω(err).ShouldNot(Succeed()) Ω(err.Error()).Should(ContainSubstring(errorMessage)) }) It("Fails to create a new load balancer rule.", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()). Return(&csapi.ListLoadBalancerRulesResponse{ @@ -398,10 +633,351 @@ var _ = Describe("Network", func() { Return(&csapi.CreateLoadBalancerRuleParams{}) lbs.EXPECT().CreateLoadBalancerRule(gomock.Any()). Return(nil, fakeError) - err := client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster) + err := client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster) Ω(err).ShouldNot(Succeed()) - Ω(err.Error()).Should(Equal(errorMessage)) + Ω(err.Error()).Should(ContainSubstring(errorMessage)) + }) + }) + + Context("The specific firewall rule exists", func() { + It("does not call create or delete firewall rule", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Cidrlist: "0.0.0.0/0", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Id: dummies.FWRuleID, + }, + }}, nil) + + fs.EXPECT().CreateFirewallRule(gomock.Any()).Times(0) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Times(0) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + + It("calls delete firewall rule when there is a rule with a cidr not in allowed cidr list", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Id: dummies.FWRuleID, + Cidrlist: "0.0.0.0/0", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + { + Id: "FakeFWRuleID2", + Cidrlist: "192.168.1.0/24", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + fs.EXPECT().NewDeleteFirewallRuleParams("FakeFWRuleID2").DoAndReturn(func(ruleid string) *csapi.DeleteFirewallRuleParams { + p := &csapi.DeleteFirewallRuleParams{} + p.SetId(ruleid) + return p + }) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Return(&csapi.DeleteFirewallRuleResponse{Success: true}, nil).Times(1) + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).Times(0) + fs.EXPECT().CreateFirewallRule(gomock.Any()).Times(0) + rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}).Times(1) + rs.EXPECT().ListTags(gomock.Any()).Return(&csapi.ListTagsResponse{ + Count: 1, + Tags: []*csapi.Tag{{ + Key: cloud.CreatedByCAPCTagName, + Value: "1", + }}, + }, nil).Times(1) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + + It("calls delete firewall rule when a port is removed from additionalPorts", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + // We pretend that port 6565 was removed from additionalPorts + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Id: dummies.FWRuleID, + Cidrlist: "0.0.0.0/0", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + { + Id: "FakeFWRuleID2", + Cidrlist: "0.0.0.0/0", + Startport: 6565, + Endport: 6565, + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + fs.EXPECT().NewDeleteFirewallRuleParams("FakeFWRuleID2").DoAndReturn(func(ruleid string) *csapi.DeleteFirewallRuleParams { + p := &csapi.DeleteFirewallRuleParams{} + p.SetId(ruleid) + return p + }) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Return(&csapi.DeleteFirewallRuleResponse{Success: true}, nil).Times(1) + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).Times(0) + fs.EXPECT().CreateFirewallRule(gomock.Any()).Times(0) + rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}).Times(1) + rs.EXPECT().ListTags(gomock.Any()).Return(&csapi.ListTagsResponse{ + Count: 1, + Tags: []*csapi.Tag{{ + Key: cloud.CreatedByCAPCTagName, + Value: "1", + }}, + }, nil).Times(1) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + }) + + Context("The specific firewall rule does not exist", func() { + It("calls create firewall rule, does not call delete firewall rule", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{}}, nil) + + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetStartport(int(dummies.EndPointPort)) + p.SetEndport(int(dummies.EndPointPort)) + p.SetProtocol(proto) + return p + }).Times(1) + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Times(0) + rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateTagsParams{}).Times(1) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(1) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + + It("calls create and delete firewall rule when there is a rule with a cidr not in allowed cidr list", func() { + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Id: "FakeFWRuleID2", + Cidrlist: "192.168.1.0/24", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + fs.EXPECT().NewDeleteFirewallRuleParams("FakeFWRuleID2").DoAndReturn(func(ruleid string) *csapi.DeleteFirewallRuleParams { + p := &csapi.DeleteFirewallRuleParams{} + p.SetId(ruleid) + return p + }).Times(1) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Return(&csapi.DeleteFirewallRuleResponse{Success: true}, nil).Times(1) + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetStartport(int(dummies.EndPointPort)) + p.SetEndport(int(dummies.EndPointPort)) + p.SetProtocol(proto) + return p + }).Times(1) + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1) + rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}).Times(1) + rs.EXPECT().ListTags(gomock.Any()).Return(&csapi.ListTagsResponse{ + Count: 1, + Tags: []*csapi.Tag{{ + Key: cloud.CreatedByCAPCTagName, + Value: "1", + }}, + }, nil).Times(1) + rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateTagsParams{}).Times(1) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(1) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + + It("calls create firewall rule 2 times with additional port, does not call delete firewall rule", func() { + dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AdditionalPorts, 456) + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{}}, nil) + + gomock.InOrder( + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetStartport(int(dummies.EndPointPort)) + p.SetEndport(int(dummies.EndPointPort)) + p.SetProtocol(proto) + return p + }).Times(1), + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1), + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetStartport(456) + p.SetEndport(456) + p.SetProtocol(proto) + return p + }).Times(1), + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1), + ) + rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateTagsParams{}).Times(2) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(2) + + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Times(0) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + + It("with a list of allowed CIDRs, calls create firewall rule for each of them, and the isonet outgoing IP, does not call delete firewall rule", func() { + dummies.CSCluster.Spec.APIServerLoadBalancer.AllowedCIDRs = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AllowedCIDRs, + "192.168.1.0/24", + "192.168.2.0/24") + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.PublicIPAddress = "10.11.12.13/32" + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{}}, nil) + + gomock.InOrder( + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetCidrlist([]string{"10.11.12.13/32"}) + p.SetStartport(int(dummies.EndPointPort)) + p.SetEndport(int(dummies.EndPointPort)) + p.SetProtocol(proto) + return p + }).Times(1), + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1), + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetCidrlist([]string{"192.168.1.0/24"}) + p.SetStartport(int(dummies.EndPointPort)) + p.SetEndport(int(dummies.EndPointPort)) + p.SetProtocol(proto) + return p + }).Times(1), + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1), + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).DoAndReturn(func(publicipid, proto string) *csapi.CreateFirewallRuleParams { + p := &csapi.CreateFirewallRuleParams{} + p.SetIpaddressid(publicipid) + p.SetCidrlist([]string{"192.168.2.0/24"}) + p.SetStartport(int(dummies.EndPointPort)) + p.SetEndport(int(dummies.EndPointPort)) + p.SetProtocol(proto) + return p + }).Times(1), + fs.EXPECT().CreateFirewallRule(gomock.Any()).Return(&csapi.CreateFirewallRuleResponse{}, nil).Times(1), + ) + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Times(0) + rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&csapi.CreateTagsParams{}).Times(3) + rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(3) + + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + }) + }) + + Context("With the API server load balancer enabled", func() { + It("Removes all firewall and load balancer rules if the API server load balancer is disabled", func() { + dummies.CSCluster.Spec.APIServerLoadBalancer.AllowedCIDRs = append(dummies.CSCluster.Spec.APIServerLoadBalancer.AllowedCIDRs, + "192.168.1.0/24", + "192.168.2.0/24") + dummies.CSCluster.Spec.APIServerLoadBalancer.Enabled = pointer.Bool(false) + dummies.CSISONet1.Status.PublicIPID = dummies.PublicIPID + dummies.CSISONet1.Status.APIServerLoadBalancer.IPAddressID = dummies.LoadBalancerIPID + + lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) + lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return( + &csapi.ListLoadBalancerRulesResponse{LoadBalancerRules: []*csapi.LoadBalancerRule{ + { + Id: dummies.LBRuleID, + Publicport: strconv.Itoa(int(dummies.EndPointPort)), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + fs.EXPECT().NewListFirewallRulesParams().Return(&csapi.ListFirewallRulesParams{}) + fs.EXPECT().ListFirewallRules(gomock.Any()).Return( + &csapi.ListFirewallRulesResponse{FirewallRules: []*csapi.FirewallRule{ + { + Id: "FakeFWRuleID1", + Cidrlist: "192.168.1.0/24", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + { + Id: "FakeFWRuleID2", + Cidrlist: "192.168.2.0/24", + Startport: int(dummies.EndPointPort), + Endport: int(dummies.EndPointPort), + Tags: dummies.CreatedByCAPCTag, + }, + }}, nil) + + gomock.InOrder( + lbs.EXPECT().NewDeleteLoadBalancerRuleParams(dummies.LBRuleID). + Return(&csapi.DeleteLoadBalancerRuleParams{}).Times(1), + lbs.EXPECT().DeleteLoadBalancerRule(gomock.Any()). + Return(&csapi.DeleteLoadBalancerRuleResponse{Success: true}, nil).Times(1), + + fs.EXPECT().NewDeleteFirewallRuleParams("FakeFWRuleID1").DoAndReturn(func(ruleid string) *csapi.DeleteFirewallRuleParams { + p := &csapi.DeleteFirewallRuleParams{} + p.SetId(ruleid) + return p + }), + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Return(&csapi.DeleteFirewallRuleResponse{Success: true}, nil).Times(1), + fs.EXPECT().NewDeleteFirewallRuleParams("FakeFWRuleID2").DoAndReturn(func(ruleid string) *csapi.DeleteFirewallRuleParams { + p := &csapi.DeleteFirewallRuleParams{} + p.SetId(ruleid) + return p + }), + fs.EXPECT().DeleteFirewallRule(gomock.Any()).Return(&csapi.DeleteFirewallRuleResponse{Success: true}, nil).Times(1), + ) + + rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}).Times(3) + rs.EXPECT().ListTags(gomock.Any()).Return(&csapi.ListTagsResponse{ + Count: 1, + Tags: []*csapi.Tag{{ + Key: cloud.CreatedByCAPCTagName, + Value: "1", + }}, + }, nil).Times(3) + + fs.EXPECT().NewCreateFirewallRuleParams(gomock.Any(), gomock.Any()).Times(0) + fs.EXPECT().CreateFirewallRule(gomock.Any()).Times(0) + Ω(client.ReconcileLoadBalancerRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(client.ReconcileFirewallRules(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(dummies.CSISONet1.Status.LoadBalancerRuleIDs).Should(Equal([]string{})) }) }) diff --git a/pkg/cloud/network.go b/pkg/cloud/network.go index dc06f4e8..9e62f858 100644 --- a/pkg/cloud/network.go +++ b/pkg/cloud/network.go @@ -37,7 +37,7 @@ const ( NetworkProtocolICMP = "icmp" ) -// ResolveNetwork fetches networks' ID, Name, and Type. +// ResolveNetwork fetches networks' ID, Name, Type and Domain. func (c *client) ResolveNetwork(net *infrav1.Network) (retErr error) { // TODO rebuild this to consider cases with networks in many zones. // Use ListNetworks instead. @@ -52,6 +52,8 @@ func (c *client) ResolveNetwork(net *infrav1.Network) (retErr error) { } else { // Got netID from the network's name. net.ID = netDetails.Id net.Type = netDetails.Type + net.CIDR = netDetails.Cidr + net.Domain = netDetails.Networkdomain return nil } @@ -66,6 +68,8 @@ func (c *client) ResolveNetwork(net *infrav1.Network) (retErr error) { net.Name = netDetails.Name net.ID = netDetails.Id net.Type = netDetails.Type + net.CIDR = netDetails.Cidr + net.Domain = netDetails.Networkdomain return nil } diff --git a/pkg/cloud/tags.go b/pkg/cloud/tags.go index a679b30b..b02b6785 100644 --- a/pkg/cloud/tags.go +++ b/pkg/cloud/tags.go @@ -39,10 +39,12 @@ type TagIface interface { type ResourceType string const ( - ClusterTagNamePrefix = "CAPC_cluster_" - CreatedByCAPCTagName = "created_by_CAPC" - ResourceTypeNetwork ResourceType = "Network" - ResourceTypeIPAddress ResourceType = "PublicIpAddress" + ClusterTagNamePrefix = "CAPC_cluster_" + CreatedByCAPCTagName = "created_by_CAPC" + ResourceTypeNetwork ResourceType = "Network" + ResourceTypeIPAddress ResourceType = "PublicIpAddress" + ResourceTypeLoadBalancerRule ResourceType = "LoadBalancer" + ResourceTypeFirewallRule ResourceType = "FirewallRule" ) // ignoreAlreadyPresentErrors returns nil if the error is an already present tag error. @@ -54,6 +56,7 @@ func ignoreAlreadyPresentErrors(err error, rType ResourceType, rID string) error return nil } +// IsCapcManaged checks whether the resource has the CreatedByCAPCTag. func (c *client) IsCapcManaged(resourceType ResourceType, resourceID string) (bool, error) { tags, err := c.GetTags(resourceType, resourceID) if err != nil { diff --git a/pkg/cloud/tags_test.go b/pkg/cloud/tags_test.go index 03bccc09..ccb07d8f 100644 --- a/pkg/cloud/tags_test.go +++ b/pkg/cloud/tags_test.go @@ -103,7 +103,7 @@ var _ = Describe("Tag Unit Tests", func() { // Verify tags tags, err := client.GetTags(cloud.ResourceTypeNetwork, dummies.CSISONet1.Spec.ID) Ω(err).Should(BeNil()) - Ω(tags[dummies.CreatedByCapcKey]).Should(Equal("")) + Ω(tags[cloud.CreatedByCAPCTagName]).Should(Equal("")) Ω(tags[dummies.CSClusterTagKey]).Should(Equal("")) }) diff --git a/pkg/utils/strings/strings.go b/pkg/utils/strings/strings.go new file mode 100644 index 00000000..91c36cdd --- /dev/null +++ b/pkg/utils/strings/strings.go @@ -0,0 +1,54 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed 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 strings + +import ( + "cmp" + "slices" +) + +func Canonicalize[S ~[]E, E cmp.Ordered](s S) S { + slices.Sort(s) + return slices.Compact(s) +} + +// SliceDiff determines the differences between two slices, returning the items in list1 that are not present in list2, +// and the items in list2 that are not present in list1. +func SliceDiff[T ~[]E, E comparable](list1 T, list2 T) (ret1 T, ret2 T) { + m1 := map[E]struct{}{} + m2 := map[E]struct{}{} + for _, v := range list1 { + m1[v] = struct{}{} + } + for _, v := range list2 { + m2[v] = struct{}{} + } + + ret1 = make(T, 0) + ret2 = make(T, 0) + for _, v := range list1 { + if _, ok := m2[v]; !ok { + ret1 = append(ret1, v) + } + } + for _, v := range list2 { + if _, ok := m1[v]; !ok { + ret2 = append(ret2, v) + } + } + return ret1, ret2 +} diff --git a/pkg/utils/strings/strings_test.go b/pkg/utils/strings/strings_test.go new file mode 100644 index 00000000..9a6bb6df --- /dev/null +++ b/pkg/utils/strings/strings_test.go @@ -0,0 +1,123 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed 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 strings + +import ( + "slices" + "testing" +) + +func TestCanonicalize(t *testing.T) { + tests := []struct { + name string + value []string + want []string + }{ + { + name: "Empty list", + value: []string{}, + want: []string{}, + }, + { + name: "Identity", + value: []string{"a", "b", "c"}, + want: []string{"a", "b", "c"}, + }, + { + name: "Out of order", + value: []string{"c", "b", "a"}, + want: []string{"a", "b", "c"}, + }, + { + name: "Duplicate elements", + value: []string{"c", "b", "a", "c"}, + want: []string{"a", "b", "c"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Canonicalize(tt.value) + if !slices.Equal(got, tt.want) { + t.Errorf("CompareLists() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSliceDiff(t *testing.T) { + tests := []struct { + name string + value1, value2 []string + want1, want2 []string + }{ + { + name: "Empty list", + value1: []string{}, + value2: []string{}, + want1: []string{}, + want2: []string{}, + }, + { + name: "Same", + value1: []string{"a", "b", "c"}, + value2: []string{"a", "b", "c"}, + want1: []string{}, + want2: []string{}, + }, + { + name: "one different", + value1: []string{"a", "b", "c"}, + value2: []string{"a", "b", "c", "d"}, + want1: []string{}, + want2: []string{"d"}, + }, + { + name: "other different", + value1: []string{"a", "b", "c", "d"}, + value2: []string{"a", "b", "c"}, + want1: []string{"d"}, + want2: []string{}, + }, + { + name: "both different", + value1: []string{"a", "b", "c", "e"}, + value2: []string{"a", "b", "d", "f"}, + want1: []string{"c", "e"}, + want2: []string{"d", "f"}, + }, + { + name: "Duplicate elements", + value1: []string{"c", "b", "a", "c"}, + value2: []string{"c", "b", "a", "c"}, + want1: []string{}, + want2: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res1, res2 := SliceDiff(tt.value1, tt.value2) + if !slices.Equal(res1, tt.want1) { + t.Errorf("CompareLists() = %v, want %v", res1, tt.want1) + } + if !slices.Equal(res2, tt.want2) { + t.Errorf("CompareLists() = %v, want %v", res2, tt.want2) + } + }) + } +} diff --git a/test/dummies/v1beta1/vars.go b/test/dummies/v1beta1/vars.go index 06d51160..8bd3ee14 100644 --- a/test/dummies/v1beta1/vars.go +++ b/test/dummies/v1beta1/vars.go @@ -71,8 +71,6 @@ var ( // Declare exported dummy vars. CSClusterTagKey string CSClusterTagVal string CSClusterTag map[string]string - CreatedByCapcKey string - CreatedByCapcVal string LBRuleID string PublicIPID string EndPointHost string @@ -134,8 +132,6 @@ func SetDummyTagVars() { CSClusterTagKey = "CAPC_cluster_" + string(CSCluster.ObjectMeta.UID) CSClusterTagVal = "1" CSClusterTag = map[string]string{CSClusterTagVal: CSClusterTagVal} - CreatedByCapcKey = "create_by_CAPC" - CreatedByCapcVal = "" Tag1Key = "test_tag1" Tag1Val = "arbitrary_value1" Tag2Key = "test_tag2" diff --git a/test/dummies/v1beta2/vars.go b/test/dummies/v1beta2/vars.go index 6e7d3d47..1adc572c 100644 --- a/test/dummies/v1beta2/vars.go +++ b/test/dummies/v1beta2/vars.go @@ -75,8 +75,6 @@ var ( // Declare exported dummy vars. CSClusterTagKey string CSClusterTagVal string CSClusterTag map[string]string - CreatedByCapcKey string - CreatedByCapcVal string LBRuleID string PublicIPID string EndPointHost string @@ -139,8 +137,6 @@ func SetDummyTagVars() { CSClusterTagKey = "CAPC_cluster_" + string(CSCluster.ObjectMeta.UID) CSClusterTagVal = "1" CSClusterTag = map[string]string{CSClusterTagVal: CSClusterTagVal} - CreatedByCapcKey = "create_by_CAPC" - CreatedByCapcVal = "" Tag1Key = "test_tag1" Tag1Val = "arbitrary_value1" Tag2Key = "test_tag2" diff --git a/test/dummies/v1beta3/vars.go b/test/dummies/v1beta3/vars.go index 2c950123..1f1ed096 100644 --- a/test/dummies/v1beta3/vars.go +++ b/test/dummies/v1beta3/vars.go @@ -2,6 +2,8 @@ package dummies import ( "os" + "path" + goruntime "runtime" csapi "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/onsi/gomega" @@ -39,6 +41,7 @@ var ( // Declare exported dummy vars. Zone2 infrav1.CloudStackZoneSpec CSFailureDomain1 *infrav1.CloudStackFailureDomain CSFailureDomain2 *infrav1.CloudStackFailureDomain + APIServerLoadBalancer *infrav1.APIServerLoadBalancer Net1 infrav1.Network Net2 infrav1.Network ISONet1 infrav1.Network @@ -75,9 +78,11 @@ var ( // Declare exported dummy vars. CSClusterTagKey string CSClusterTagVal string CSClusterTag map[string]string - CreatedByCapcKey string - CreatedByCapcVal string + CreatedByCAPCTag []csapi.Tags + FWRuleID string LBRuleID string + LoadBalancerRuleIDs []string + LoadBalancerIPID string PublicIPID string EndPointHost string EndPointPort int32 @@ -91,8 +96,10 @@ var ( // Declare exported dummy vars. // SetDummyVars sets/resets all dummy vars. func SetDummyVars() { - projDir := os.Getenv("REPO_ROOT") - source, err := os.ReadFile(projDir + "/test/e2e/config/cloudstack.yaml") + // Get the root of the current file to use in CRD paths. + _, filename, _, _ := goruntime.Caller(0) //nolint:dogsled // Ignore "declaration has 3 blank identifiers" check. + root := path.Join(path.Dir(filename), "..", "..") + source, err := os.ReadFile(root + "/e2e/config/cloudstack.yaml") if err != nil { panic(err) } @@ -114,7 +121,9 @@ func SetDummyVars() { SetDummyBootstrapSecretVar() SetCSMachineOwner() SetDummyOwnerReferences() + FWRuleID = "FakeFWRuleID" LBRuleID = "FakeLBRuleID" + LoadBalancerRuleIDs = []string{"FakeLBRuleID"} } func SetDiskOfferingVars() { @@ -139,8 +148,6 @@ func SetDummyTagVars() { CSClusterTagKey = "CAPC_cluster_" + string(CSCluster.ObjectMeta.UID) CSClusterTagVal = "1" CSClusterTag = map[string]string{CSClusterTagVal: CSClusterTagVal} - CreatedByCapcKey = "create_by_CAPC" - CreatedByCapcVal = "" Tag1Key = "test_tag1" Tag1Val = "arbitrary_value1" Tag2Key = "test_tag2" @@ -148,6 +155,12 @@ func SetDummyTagVars() { Tag1 = map[string]string{Tag2Key: Tag2Val} Tag2 = map[string]string{Tag2Key: Tag2Val} Tags = map[string]string{Tag1Key: Tag1Val, Tag2Key: Tag2Val} + CreatedByCAPCTag = []csapi.Tags{ + { + Key: cloud.CreatedByCAPCTagName, + Value: "1", + }, + } } func SetCSMachineOwner() { @@ -274,6 +287,7 @@ func SetDummyCAPCClusterVars() { ClusterName = "test-cluster" EndPointHost = "EndpointHost" EndPointPort = int32(5309) + LoadBalancerIPID = "FakeLoadBalancerPublicIPID" PublicIPID = "FakePublicIPID" ClusterNameSpace = "default" ClusterLabel = map[string]string{clusterv1.ClusterNameLabel: ClusterName} @@ -284,6 +298,11 @@ func SetDummyCAPCClusterVars() { Net1 = infrav1.Network{Name: GetYamlVal("CLOUDSTACK_NETWORK_NAME"), Type: cloud.NetworkTypeShared} Net2 = infrav1.Network{Name: "SharedGuestNet2", Type: cloud.NetworkTypeShared, ID: "FakeSharedNetID2"} ISONet1 = infrav1.Network{Name: "isoguestnet1", Type: cloud.NetworkTypeIsolated, ID: "FakeIsolatedNetID1"} + APIServerLoadBalancer = &infrav1.APIServerLoadBalancer{ + Enabled: pointer.Bool(true), + AdditionalPorts: []int{}, + AllowedCIDRs: []string{}, + } CSFailureDomain1 = &infrav1.CloudStackFailureDomain{ TypeMeta: metav1.TypeMeta{ APIVersion: CSApiVersion, @@ -335,8 +354,9 @@ func SetDummyCAPCClusterVars() { Labels: ClusterLabel, }, Spec: infrav1.CloudStackClusterSpec{ - ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: EndPointHost, Port: EndPointPort}, - FailureDomains: []infrav1.CloudStackFailureDomainSpec{CSFailureDomain1.Spec, CSFailureDomain2.Spec}, + ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: EndPointHost, Port: EndPointPort}, + FailureDomains: []infrav1.CloudStackFailureDomainSpec{CSFailureDomain1.Spec, CSFailureDomain2.Spec}, + APIServerLoadBalancer: APIServerLoadBalancer, }, Status: infrav1.CloudStackClusterStatus{}, } @@ -352,7 +372,12 @@ func SetDummyCAPCClusterVars() { Labels: ClusterLabel, }, Spec: infrav1.CloudStackIsolatedNetworkSpec{ - ControlPlaneEndpoint: CSCluster.Spec.ControlPlaneEndpoint}} + ControlPlaneEndpoint: CSCluster.Spec.ControlPlaneEndpoint, + }, + Status: infrav1.CloudStackIsolatedNetworkStatus{ + APIServerLoadBalancer: &infrav1.LoadBalancer{}, + }, + } CSISONet1.Spec.Name = ISONet1.Name CSISONet1.Spec.ID = ISONet1.ID } diff --git a/test/e2e/go.mod b/test/e2e/go.mod index 1e96e51c..210d45e0 100644 --- a/test/e2e/go.mod +++ b/test/e2e/go.mod @@ -1,6 +1,6 @@ module sigs.k8s.io/cluster-api-provider-cloudstack-staging/test/e2e -go 1.21 +go 1.22 require ( github.com/Shopify/toxiproxy/v2 v2.5.0 @@ -9,8 +9,8 @@ require ( github.com/onsi/ginkgo/v2 v2.19.0 github.com/onsi/gomega v1.33.1 gopkg.in/yaml.v3 v3.0.1 - k8s.io/api v0.27.14 - k8s.io/apimachinery v0.27.14 + k8s.io/api v0.27.16 + k8s.io/apimachinery v0.27.16 k8s.io/klog/v2 v2.90.1 k8s.io/utils v0.0.0-20230711102312-30195339c3c7 sigs.k8s.io/cluster-api v1.5.8 @@ -115,11 +115,11 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - k8s.io/apiextensions-apiserver v0.27.14 // indirect - k8s.io/apiserver v0.27.14 // indirect - k8s.io/client-go v0.27.14 // indirect - k8s.io/cluster-bootstrap v0.27.2 // indirect - k8s.io/component-base v0.27.14 // indirect + k8s.io/apiextensions-apiserver v0.27.16 // indirect + k8s.io/apiserver v0.27.16 // indirect + k8s.io/client-go v0.27.16 // indirect + k8s.io/cluster-bootstrap v0.27.16 // indirect + k8s.io/component-base v0.27.16 // indirect k8s.io/kube-openapi v0.0.0-20230601164746-7562a1006961 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kind v0.20.0 // indirect diff --git a/test/e2e/go.sum b/test/e2e/go.sum index 2c110137..32a1974a 100644 --- a/test/e2e/go.sum +++ b/test/e2e/go.sum @@ -892,20 +892,20 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -k8s.io/api v0.27.14 h1:/oKAF9HiSB47polol2Ji2TaFnC400JK57jSPUXY5MzU= -k8s.io/api v0.27.14/go.mod h1:Jekhd9Kyo2CsmJlYbqZPXNwIxiHvyGJCdp0X56yDyvU= -k8s.io/apiextensions-apiserver v0.27.14 h1:d7GtKY9F0drdgAxLnKHgAzUBXQVqyDlkjDHUHXr4B2w= -k8s.io/apiextensions-apiserver v0.27.14/go.mod h1:+i082aG+ZX89DlApNxVWj8U0VqkbvHmTF/KjpzzHhLU= -k8s.io/apimachinery v0.27.14 h1:jAIGvPbvAg4XJysK7JPFa6DdjTR6vts4/p4Q6ZrcQ+4= -k8s.io/apimachinery v0.27.14/go.mod h1:TWo+8wOIz3CytsrlI9k/LBWXLRr9dqf5hRSCbbggMAg= -k8s.io/apiserver v0.27.14 h1:qBCxFtCjKlokT6Bn6YtjvfiquU/3cOA4Pv0Yqw5BsiM= -k8s.io/apiserver v0.27.14/go.mod h1:/QZ7+YwuOuybRvRWamIUUZnYnb1SHHSAlLFlmaZUrvM= -k8s.io/client-go v0.27.14 h1:5KwfSakOTQFRlPru2Ql/wp1URjPgzoP7QpTlEH9a+ys= -k8s.io/client-go v0.27.14/go.mod h1:cy+p3ijvbPQpdcwg01qnHBmkYDtbOatNC83anA9y18g= -k8s.io/cluster-bootstrap v0.27.2 h1:OL3onrOwrUD7NQxBUqQwTl1Uu2GQKCkw9BMHpc4PbiA= -k8s.io/cluster-bootstrap v0.27.2/go.mod h1:b++PF0mjUOiTKdPQFlDw7p4V2VquANZ8SfhAwzxZJFM= -k8s.io/component-base v0.27.14 h1:Pdl5bL1TX/MtXsIwUtSctccgZRmrao6GCV3+qfxw0Qs= -k8s.io/component-base v0.27.14/go.mod h1:eiXBPnqBoczGNS09AtEWxN3Bpj4mI32FGb54S+JAcPg= +k8s.io/api v0.27.16 h1:70IBoTuiPfd+Tm68WH0tGXQRSQq0R1xnbyhTRe8WYQY= +k8s.io/api v0.27.16/go.mod h1:5j0Cgo6X4qovBOu3OjzRwETDEYqMxq2qafhDQXOPy3A= +k8s.io/apiextensions-apiserver v0.27.16 h1:gJ0sEbfYmvgdysC2WjkeYujvjmWAyPH6e8ANVAL5qxk= +k8s.io/apiextensions-apiserver v0.27.16/go.mod h1:wq5IgoFVjYyJqqcjD+R+/opZJxBQcu9PIcFWJ8eaQLQ= +k8s.io/apimachinery v0.27.16 h1:Nmbei3P/6w6vxbNxV8/sDCZz+TQrJ9A4+bVIRjDufuM= +k8s.io/apimachinery v0.27.16/go.mod h1:TWo+8wOIz3CytsrlI9k/LBWXLRr9dqf5hRSCbbggMAg= +k8s.io/apiserver v0.27.16 h1:s3+lMqISTj5l/ZH/BvhdbiMfIoTF3/lrAN99BHccLmk= +k8s.io/apiserver v0.27.16/go.mod h1:xwxM8/bcAtgkWqbsGwMQjImIC5Jik7a4pHRptEDqQf0= +k8s.io/client-go v0.27.16 h1:x06Jk6/SIQQ6kAsWs5uzQIkBLHtcAQlbTAgmj1tZzG0= +k8s.io/client-go v0.27.16/go.mod h1:bPZUNRj8XsHa+JVS5jU6qeU2H/Za8+7riWA08FUjaA8= +k8s.io/cluster-bootstrap v0.27.16 h1:iySGnST9X4W1IfAdANdF6uBzV6kTL9SIAiKQnlZm4ug= +k8s.io/cluster-bootstrap v0.27.16/go.mod h1:u7tVB3+r4X0I/fEfirH+dNWuwGjQyReAVZa7/V92Pkk= +k8s.io/component-base v0.27.16 h1:CpPBD1GIwsaRdDF0WzJkIppakYJwQCvsKK8exRxe9rY= +k8s.io/component-base v0.27.16/go.mod h1:g636fljq9A7zsIB0nRE4fgmBCo8aqjoJe1aLkCX0Vwc= k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw= k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20230601164746-7562a1006961 h1:pqRVJGQJz6oeZby8qmPKXYIBjyrcv7EHCe/33UkZMYA= diff --git a/version/version.go b/version/version.go new file mode 100644 index 00000000..322cbd1a --- /dev/null +++ b/version/version.go @@ -0,0 +1,62 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed 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 version + +import ( + "fmt" + "runtime" +) + +var ( + gitMajor string // major version, always numeric + gitMinor string // minor version, numeric possibly followed by "+" + gitVersion string // semantic version, derived by build scripts + gitCommit string // sha1 from git, output of $(git rev-parse HEAD) + gitTreeState string // state of git tree, either "clean" or "dirty" + buildDate string // build date in ISO8601 format, output of $(date -u +'%Y-%m-%dT%H:%M:%SZ') +) + +type Info struct { + Major string `json:"major,omitempty"` + Minor string `json:"minor,omitempty"` + GitVersion string `json:"gitVersion,omitempty"` + GitCommit string `json:"gitCommit,omitempty"` + GitTreeState string `json:"gitTreeState,omitempty"` + BuildDate string `json:"buildDate,omitempty"` + GoVersion string `json:"goVersion,omitempty"` + Compiler string `json:"compiler,omitempty"` + Platform string `json:"platform,omitempty"` +} + +func Get() Info { + return Info{ + Major: gitMajor, + Minor: gitMinor, + GitVersion: gitVersion, + GitCommit: gitCommit, + GitTreeState: gitTreeState, + BuildDate: buildDate, + GoVersion: runtime.Version(), + Compiler: runtime.Compiler, + Platform: fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH), + } +} + +// String returns info as a human-friendly version string. +func (info Info) String() string { + return info.GitVersion +}