From 42e485e51feafd045cf626376725649df6a0afce Mon Sep 17 00:00:00 2001 From: Pierre Ozoux Date: Fri, 27 Sep 2024 17:59:11 +0200 Subject: [PATCH 01/11] fix: typo on omi --- docs/src/topics/omi.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/src/topics/omi.md b/docs/src/topics/omi.md index 40429cde9..fa3e08dc4 100644 --- a/docs/src/topics/omi.md +++ b/docs/src/topics/omi.md @@ -10,12 +10,12 @@ New OMI will be builded and publish every two weeks for each supported Os Distri - us-east-2 - -## Supported Image on eu-east-2: +## Supported Image on eu-west-2: ubuntu: -- ubuntu-2204-2204-kubernetes-v1.28.5-2024-01-17 -- ubuntu-2204-2204-kubernetes-v1.27.9-2024-01-17 -- ubuntu-2204-2204-kubernetes-v1.26.12-2024-01-17 -- ubuntu-2204-2204-kubernetes-v1.25.16-2024-01-17 +- ubuntu-2204-2204-kubernetes-v1.28.5-2024-01-10 +- ubuntu-2204-2204-kubernetes-v1.27.9-2024-01-10 +- ubuntu-2204-2204-kubernetes-v1.26.12-2024-01-10 +- ubuntu-2204-2204-kubernetes-v1.25.16-2024-01-10 - ubuntu-2204-2204-kubernetes-v1.24.16-2024-01-23 - ubuntu-2204-2204-kubernetes-v1.23.17-2024-01-14 - ubuntu-2204-2204-kubernetes-v1.22.11-2024-01-17 From 0c1a8d7276da89639dc63287e079ceaf1b71c3ba Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Tue, 19 Nov 2024 15:41:32 +0000 Subject: [PATCH 02/11] upgrade sdk go Signed-off-by: hanenMizouni --- controllers/osccluster_loadbalancer_controller.go | 2 +- controllers/osccluster_loadbalancer_controller_unit_test.go | 6 +++--- go.mod | 2 +- go.sum | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/controllers/osccluster_loadbalancer_controller.go b/controllers/osccluster_loadbalancer_controller.go index 623d91118..ba0f0e858 100644 --- a/controllers/osccluster_loadbalancer_controller.go +++ b/controllers/osccluster_loadbalancer_controller.go @@ -266,7 +266,7 @@ func reconcileDeleteLoadBalancer(ctx context.Context, clusterScope *scope.Cluste } loadBalancerTagKey := osc.ResourceLoadBalancerTag{ - Key: &nameTag.Key, + Key: nameTag.Key, } err = loadBalancerSvc.DeleteLoadBalancerTag(loadBalancerSpec, loadBalancerTagKey) if err != nil { diff --git a/controllers/osccluster_loadbalancer_controller_unit_test.go b/controllers/osccluster_loadbalancer_controller_unit_test.go index fd2134406..e8c0daf4d 100644 --- a/controllers/osccluster_loadbalancer_controller_unit_test.go +++ b/controllers/osccluster_loadbalancer_controller_unit_test.go @@ -1190,7 +1190,7 @@ func TestReconcileDeleteLoadBalancerDelete(t *testing.T) { } if lbtc.expDeleteLoadBalancerTagFound { loadBalancerTagKey := osc.ResourceLoadBalancerTag{ - Key: tag.Key, + Key: *tag.Key, } mockOscLoadBalancerInterface. EXPECT(). @@ -1338,7 +1338,7 @@ func TestReconcileDeleteLoadBalancerDeleteTag(t *testing.T) { } if lbtc.expDeleteLoadBalancerTagFound { loadBalancerTagKey := osc.ResourceLoadBalancerTag{ - Key: tag.Key, + Key: *tag.Key, } mockOscLoadBalancerInterface. EXPECT(). @@ -1450,7 +1450,7 @@ func TestReconcileDeleteLoadBalancerDeleteWithoutSpec(t *testing.T) { } if lbtc.expDeleteLoadBalancerTagFound { loadBalancerTagKey := osc.ResourceLoadBalancerTag{ - Key: tag.Key, + Key: *tag.Key, } mockOscLoadBalancerInterface. EXPECT(). diff --git a/go.mod b/go.mod index cf8f31410..9d2722c80 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/golang/mock v1.6.0 github.com/onsi/ginkgo/v2 v2.19.1 github.com/onsi/gomega v1.34.0 - github.com/outscale/osc-sdk-go/v2 v2.21.0 + github.com/outscale/osc-sdk-go/v2 v2.24.0 github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.9.0 golang.org/x/exp v0.0.0-20231006140011-7918f672742d diff --git a/go.sum b/go.sum index 8bf028887..3c56e3682 100644 --- a/go.sum +++ b/go.sum @@ -201,8 +201,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.2 h1:9yCKha/T5XdGtO0q9Q9a6T5NUCsTn/DrBg0D7ufOcFM= github.com/opencontainers/image-spec v1.0.2/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= -github.com/outscale/osc-sdk-go/v2 v2.21.0 h1:8MqhexV+ALG76CvY8EpuQlr0+hC5VFrWQY4fHqdT358= -github.com/outscale/osc-sdk-go/v2 v2.21.0/go.mod h1:kzhtUErCzKYl87bZ+kDMphDafmnwbsyFJY9iHF7NgNE= +github.com/outscale/osc-sdk-go/v2 v2.24.0 h1:4M0gJgYRKJQhIo5oZbiEbfuBv/Ls6iHaUyrEiBAbjYM= +github.com/outscale/osc-sdk-go/v2 v2.24.0/go.mod h1:kzhtUErCzKYl87bZ+kDMphDafmnwbsyFJY9iHF7NgNE= github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8= github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c= github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6Wq+LM= From b884c2c90578888dd44c465d0aaec1ed78cc23ad Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Wed, 20 Nov 2024 13:07:13 +0000 Subject: [PATCH 03/11] add clean up script to ensure deletion of crds Signed-off-by: hanenMizouni --- .github/scripts/cleanup_k8s_crds.sh | 74 +++++++++++++++++++++++ .github/workflows/unit-func-e2e-test.yaml | 7 +++ 2 files changed, 81 insertions(+) create mode 100644 .github/scripts/cleanup_k8s_crds.sh diff --git a/.github/scripts/cleanup_k8s_crds.sh b/.github/scripts/cleanup_k8s_crds.sh new file mode 100644 index 000000000..a1599dd29 --- /dev/null +++ b/.github/scripts/cleanup_k8s_crds.sh @@ -0,0 +1,74 @@ +#!/bin/bash + +set -e + +# Function to patch resources to remove finalizers +remove_finalizers() { + local resource_type=$1 + local resource_name=$2 + + echo "Removing finalizers from $resource_type/$resource_name (if any)..." + kubectl patch "$resource_type" "$resource_name" --type='merge' -p '{"metadata":{"finalizers":[]}}' || echo "No finalizers to patch or resource does not exist." +} + +# Function to delete all resources of a given CRD type +delete_resources() { + local resource_type=$1 + + echo "Deleting all resources of type $resource_type..." + kubectl delete "$resource_type" --all --ignore-not-found || echo "No resources of type $resource_type found." +} + +# Function to delete a CRD +delete_crd() { + local crd_name=$1 + + echo "Deleting CRD $crd_name..." + kubectl delete crd "$crd_name" --ignore-not-found || echo "CRD $crd_name not found." +} + +# Main cleanup logic +cleanup_crd() { + local crd_name=$1 + local resource_name=$2 + + echo "Starting cleanup for CRD $crd_name and resource $resource_name..." + + # Remove finalizers from the resource (if exists) + if [ -n "$resource_name" ]; then + remove_finalizers "$crd_name" "$resource_name" + fi + + # Delete all resources associated with the CRD + delete_resources "$crd_name" + + # Delete the CRD itself + delete_crd "$crd_name" + + echo "Cleanup for $crd_name complete." +} + +# List of CRDs to clean up (add more as needed) +CRD_LIST=( + "oscclusters.infrastructure.cluster.x-k8s.io" + # Add more CRDs here if needed +) + +# List of specific resources to patch/remove finalizers (CRD/resource name pairs) +RESOURCE_LIST=( + "oscclusters.infrastructure.cluster.x-k8s.io/cluster-api-test" + # Add more resources here if needed in the format "crd/resource_name" +) + +# Perform cleanup for each resource in the RESOURCE_LIST +for resource_entry in "${RESOURCE_LIST[@]}"; do + IFS="/" read -r crd resource <<< "$resource_entry" + cleanup_crd "$crd" "$resource" +done + +# Perform cleanup for all CRDs in the CRD_LIST (general cleanup) +for crd in "${CRD_LIST[@]}"; do + cleanup_crd "$crd" "" +done + +echo "Kubernetes CRD cleanup complete." \ No newline at end of file diff --git a/.github/workflows/unit-func-e2e-test.yaml b/.github/workflows/unit-func-e2e-test.yaml index fa9502a5a..96e8d0098 100644 --- a/.github/workflows/unit-func-e2e-test.yaml +++ b/.github/workflows/unit-func-e2e-test.yaml @@ -177,6 +177,13 @@ jobs: env: KUBECONFIG: rke-cluster-for-cluster-api/rke/kube_config_cluster.yml CAPO_NAMESPACE: cluster-api-provider-outscale-system + - name: Run Cleanup Script + run: | + chmod +x .github/scripts/cleanup_k8s_crds.sh + .github/scripts/cleanup_k8s_crds.sh + shell: bash + env: + KUBECONFIG: ${{ github.workspace }}/rke-cluster-for-cluster-api/rke/kube_config_cluster.yml - name: Destroy cluster uses: ./rke-cluster-for-cluster-api/github_actions/destroy_cluster/ if: ${{ always() }} From aa4ebc9ce39710d6caa42a6a75d990f13bfa3d0a Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Tue, 19 Nov 2024 21:46:00 +0000 Subject: [PATCH 04/11] docs: Add example for Cluster API with MetalLB integration Signed-off-by: hanenMizouni --- example/metalLb/Readme.md | 63 +++++++++++ example/metalLb/metalLb.yaml | 211 +++++++++++++++++++++++++++++++++++ example/metalLb/service.yaml | 32 ++++++ 3 files changed, 306 insertions(+) create mode 100644 example/metalLb/Readme.md create mode 100644 example/metalLb/metalLb.yaml create mode 100644 example/metalLb/service.yaml diff --git a/example/metalLb/Readme.md b/example/metalLb/Readme.md new file mode 100644 index 000000000..6b1249086 --- /dev/null +++ b/example/metalLb/Readme.md @@ -0,0 +1,63 @@ +# Kubernetes Cluster Deployment with MetalLB + +## Prerequisites +Before starting, ensure the following are in place: + +1. Infrastructure +Management Cluster: +- A management cluster is required to use Cluster API for provisioning and managing the workload cluster. This cluster can be deployed locally (e.g., using kind or minikube) or [rke.](https://github.com/outscale/osc-k8s-rke-cluster) + +2. Tools +- kubectl +- Cluster API +- Cluster-api outscale provider + +3. MetalLB +Ensure Layer 2 (L2) connectivity between your cluster nodes to support MetalLB's ARP-based IP advertising. +IP range configured for MetalLB should not overlap with any existing subnet or DHCP ranges in your environment. + +## This documentation provides a step-by-step guide to: + +- Deploy a Kubernetes cluster with Cluster API. +- Install and configure MetalLB. +- Test the setup with a LoadBalancer service. +- Verify the assigned IP from the MetalLB IP pool. + +### MetalLB Integration in the Control Plane + +The MetalLB installation is fully automated through the postKubeadmCommands in the control plane configuration. +The IP pool (10.0.1.240-10.0.1.250) and Layer 2 advertisement configuration are pre-created as a file and applied during the node initialization. +After deployment, verify the metallb-system namespace and pods, then test by deploying a LoadBalancer service. + +```bash +kubectl apply -f example/metalLb/service.yaml +``` + +```bash +kubectl get pods -n metallb-system +``` + +```bash +NAME READY STATUS RESTARTS AGE +controller-7bcd9b5f47-l9r96 1/1 Running 0 104s +speaker-5dvs2 1/1 Running 0 104s +speaker-nnwdg 1/1 Running 0 104s +speaker-rvkmp 1/1 Running 0 104s +``` + +### Deploy a Test Service +```bash +kubectl apply -f service.yaml +``` + +```bash +kubectl get svc nginx-service +NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE +nginx-service LoadBalancer 10.43.209.200 10.0.1.244 80:30509/TCP 6m45s +``` + +#### Test the Service +Access the service using the external IP: +```bash +curl http://10.0.1.244 +``` diff --git a/example/metalLb/metalLb.yaml b/example/metalLb/metalLb.yaml new file mode 100644 index 000000000..2bfc99278 --- /dev/null +++ b/example/metalLb/metalLb.yaml @@ -0,0 +1,211 @@ +# Cluster Configuration +apiVersion: cluster.x-k8s.io/v1beta1 +kind: Cluster +metadata: + name: hello-osc + namespace: default +spec: + clusterNetwork: + pods: + cidrBlocks: + - 10.42.0.0/16 + services: + cidrBlocks: + - 10.96.0.0/12 + controlPlaneRef: + apiVersion: controlplane.cluster.x-k8s.io/v1beta1 + kind: KubeadmControlPlane + name: hello-osc-control-plane + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 + kind: OscCluster + name: hello-osc +--- +# Outscale Cluster Infrastructure +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: OscCluster +metadata: + name: hello-osc + namespace: default +spec: + network: + clusterName: hello-osc + subregionName: eu-west-2a + internetService: + clusterName: hello-osc + loadBalancer: + clusterName: hello-osc + loadbalancername: hello-osc-k8s + natService: + clusterName: hello-osc + net: + clusterName: hello-osc +--- +# Control Plane Configuration +apiVersion: controlplane.cluster.x-k8s.io/v1beta1 +kind: KubeadmControlPlane +metadata: + name: hello-osc-control-plane + namespace: default +spec: + kubeadmConfigSpec: + files: + - content: | + #!/bin/sh + + # Installer une version mise à jour de runc + curl https://github.com/opencontainers/runc/releases/download/v1.1.1/runc.amd64 -Lo /tmp/runc.amd64 + chmod +x /tmp/runc.amd64 + cp -f /tmp/runc.amd64 /usr/local/sbin/runc + + # Configurer MetalLB + cat < /tmp/metallb-config.yaml + apiVersion: metallb.io/v1beta1 + kind: IPAddressPool + metadata: + name: metallb-pool + namespace: metallb-system + spec: + addresses: + - 10.0.1.240-10.0.1.250 + --- + apiVersion: metallb.io/v1beta1 + kind: L2Advertisement + metadata: + name: l2-advertisement + namespace: metallb-system + spec: {} + EOF + owner: root:root + path: /tmp/set_runc_and_metallb.sh + permissions: "0744" + initConfiguration: + nodeRegistration: + kubeletExtraArgs: + cloud-provider: external + provider-id: aws://'{{ ds.meta_data.placement.availability_zone }}'/'{{ ds.meta_data.instance_id }}' + name: '{{ ds.meta_data.local_hostname }}' + joinConfiguration: + nodeRegistration: + kubeletExtraArgs: + cloud-provider: external + provider-id: aws://'{{ ds.meta_data.placement.availability_zone }}'/'{{ ds.meta_data.instance_id }}' + preKubeadmCommands: + - sh /tmp/set_runc_and_metallb.sh + postKubeadmCommands: + - kubectl apply -f https://raw.githubusercontent.com/metallb/metallb/main/config/manifests/metallb-native.yaml + - kubectl create namespace metallb-system || true + - kubectl apply -f /tmp/metallb-config.yaml + machineTemplate: + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 + kind: OscMachineTemplate + name: hello-osc-control-plane + replicas: 1 + version: v1.28.5 +--- +# Worker Nodes Configuration +apiVersion: cluster.x-k8s.io/v1beta1 +kind: MachineDeployment +metadata: + name: hello-osc-md-0 + namespace: default +spec: + clusterName: hello-osc + replicas: 1 + selector: + matchLabels: null + template: + spec: + bootstrap: + configRef: + apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 + kind: KubeadmConfigTemplate + name: hello-osc-md-0 + clusterName: hello-osc + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 + kind: OscMachineTemplate + name: hello-osc-md-0 + version: v1.28.5 +--- +# Worker Nodes Template +apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 +kind: KubeadmConfigTemplate +metadata: + name: hello-osc-md-0 + namespace: default +spec: + template: + spec: + files: + - content: | + #!/bin/sh + + # Installer une version mise à jour de runc + curl https://github.com/opencontainers/runc/releases/download/v1.1.1/runc.amd64 -Lo /tmp/runc.amd64 + chmod +x /tmp/runc.amd64 + cp -f /tmp/runc.amd64 /usr/local/sbin/runc + owner: root:root + path: /tmp/set_runc.sh + permissions: "0744" + joinConfiguration: + nodeRegistration: + kubeletExtraArgs: + cloud-provider: external + provider-id: aws://'{{ ds.meta_data.placement.availability_zone }}'/'{{ ds.meta_data.instance_id }}' + name: '{{ ds.meta_data.local_hostname }}' + preKubeadmCommands: + - sh /tmp/set_runc.sh +--- +# Outscale Machine Template for Control Plane +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: OscMachineTemplate +metadata: + name: hello-osc-control-plane-v2 + namespace: default +spec: + template: + spec: + node: + clusterName: hello-osc + image: + name: ubuntu-2004-2004-kubernetes-v1.28.5-2022-08-22 + keypair: + name: cluster-api-test + vm: + clusterName: hello-osc + keypairName: cluster-api-test + loadBalancerName: hello-osc-k8s + role: controlplane + rootDisk: + rootDiskIops: 2000 + rootDiskSize: 20 + rootDiskType: io1 + subregionName: eu-west-2a + vmType: tinav6.c4r8p1 +--- +# Outscale Machine Template for Workers +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: OscMachineTemplate +metadata: + name: hello-osc-md-0-v2 + namespace: default +spec: + template: + spec: + node: + clusterName: hello-osc + image: + name: ubuntu-2004-2004-kubernetes-v1.28.5-2022-08-22 + keypair: + name: cluster-api-test + vm: + clusterName: hello-osc + keypairName: cluster-api-test + rootDisk: + rootDiskIops: 2000 + rootDiskSize: 20 + rootDiskType: io1 + subregionName: eu-west-2a + vmType: tinav6.c4r8p1 \ No newline at end of file diff --git a/example/metalLb/service.yaml b/example/metalLb/service.yaml new file mode 100644 index 000000000..46569de5c --- /dev/null +++ b/example/metalLb/service.yaml @@ -0,0 +1,32 @@ +apiVersion: v1 +kind: Service +metadata: + name: nginx-service +spec: + selector: + app: nginx + ports: + - protocol: TCP + port: 80 + targetPort: 80 + type: LoadBalancer +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + replicas: 1 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + spec: + containers: + - name: nginx + image: nginx:latest + ports: + - containerPort: 80 From 7a9f1f89ed345a263598bb79b03add2edca57f76 Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Wed, 20 Nov 2024 14:43:56 +0000 Subject: [PATCH 05/11] add an example for eim policy for capi controllers Signed-off-by: hanenMizouni --- eim-policy.example.json | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 eim-policy.example.json diff --git a/eim-policy.example.json b/eim-policy.example.json new file mode 100644 index 000000000..d2cea8391 --- /dev/null +++ b/eim-policy.example.json @@ -0,0 +1,62 @@ +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "api:CreateInternetService", + "api:CreateLoadBalancer", + "api:CreateLoadBalancerTags", + "api:CreateNatService", + "api:CreateNet", + "api:CreatePublicIp", + "api:CreateRoute", + "api:CreateRouteTable", + "api:CreateSecurityGroup", + "api:CreateSecurityGroupRule", + "api:CreateSubnet", + "api:CreateVms", + "api:CreateVolume", + "api:DeleteInternetService", + "api:DeleteLoadBalancer", + "api:DeleteLoadBalancerTags", + "api:DeleteNatService", + "api:DeleteNet", + "api:DeletePublicIp", + "api:DeleteRoute", + "api:DeleteRouteTable", + "api:DeleteSecurityGroup", + "api:DeleteSecurityGroupRule", + "api:DeleteSubnet", + "api:DeleteVms", + "api:DeleteVolume", + "api:GetApi", + "api:LinkInternetService", + "api:LinkLoadBalancerBackendMachines", + "api:LinkPublicIp", + "api:LinkRouteTable", + "api:LinkVolume", + "api:ReadImages", + "api:ReadInternetServices", + "api:ReadLoadBalancerTags", + "api:ReadLoadBalancers", + "api:ReadNatServices", + "api:ReadNets", + "api:ReadPublicIps", + "api:ReadRouteTables", + "api:ReadSecurityGroups", + "api:ReadSubnets", + "api:ReadTags", + "api:ReadVms", + "api:ReadVolumes", + "api:UnlinkInternetService", + "api:UnlinkLoadBalancerBackendMachines", + "api:UnlinkPublicIp", + "api:UnlinkRouteTable", + "api:UnlinkVolume", + "api:UpdateLoadBalancer" + ], + "Resource": "*" + } + ] + } \ No newline at end of file From e373b571646d6b76b46ebfb0d5b93b58580018a0 Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Tue, 12 Nov 2024 15:18:17 +0000 Subject: [PATCH 06/11] update trivy scan db cache Signed-off-by: hanenMizouni --- .github/workflows/docker.yaml | 76 +++++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 22 deletions(-) diff --git a/.github/workflows/docker.yaml b/.github/workflows/docker.yaml index 85871e872..cd9575345 100644 --- a/.github/workflows/docker.yaml +++ b/.github/workflows/docker.yaml @@ -33,29 +33,61 @@ on: - "!hack/json-format/Makefile" - "!hack/json-format/Cargo.*" - "!hack/json-format/tests/*.rs" + jobs: docker: runs-on: ubuntu-latest + steps: - - uses: actions/checkout@v3 - - name: Docker Lint - run: bash -c "make dockerlint" - - name: Build and Push Docker image - run: | - make docker-buildx - env: - IMG: cluster-api-outscale-controller:${{ github.sha }} - DOCKER_BUILDKIT: 1 - - name: Trivy-Scan - run: bash -c "make trivy-scan" - env: - IMG: cluster-api-outscale-controller:${{ github.sha }} - - name: Trivy-Ignore-Check - run: bash -c "make trivy-ignore-check" - env: - IMG: cluster-api-outscale-controller:${{ github.sha }} - - name: Upload Scan if errors - if: ${{ always() && github.event_name != 'pull_request' }} - uses: github/codeql-action/upload-sarif@v2 - with: - sarif_file: './.trivyscan/report.sarif' + - uses: actions/checkout@v3 + + # Cache Trivy Database + - name: Cache Trivy DB + uses: actions/cache@v2 + with: + path: ~/.cache/trivy/db # This is where Trivy DB will be cached locally + key: ${{ runner.os }}-trivy-db # Unique cache key based on OS + restore-keys: | + ${{ runner.os }}-trivy-db # Fallback key if the exact cache key isn't available + + # Download Trivy DB only if cache is missing or outdated + - name: Download Trivy DB + run: | + docker run --rm \ + -v $HOME/.cache/trivy/db:/root/.cache/trivy/db \ + aquasec/trivy:latest image --download-db-only + + # Docker Lint + - name: Docker Lint + run: bash -c "make dockerlint" + + # Build and Push Docker Image + - name: Build and Push Docker Image + run: | + make docker-buildx + env: + IMG: cluster-api-outscale-controller:${{ github.sha }} + DOCKER_BUILDKIT: 1 + + # Trivy Scan + - name: Trivy Scan + run: bash -c "make trivy-scan" + env: + IMG: cluster-api-outscale-controller:${{ github.sha }} + # Mount cached Trivy DB to avoid redundant downloads + with: + args: | + -v $HOME/.cache/trivy/db:/root/.cache/trivy/db + + # Trivy Ignore Check + - name: Trivy Ignore Check + run: bash -c "make trivy-ignore-check" + env: + IMG: cluster-api-outscale-controller:${{ github.sha }} + + # Upload Trivy SARIF report if errors + - name: Upload Scan if Errors + if: ${{ always() && github.event_name != 'pull_request' }} + uses: github/codeql-action/upload-sarif@v2 + with: + sarif_file: './.trivyscan/report.sarif' \ No newline at end of file From f3ef74c2c4c8b93a7c7d2040bf21ab5eb4b86677 Mon Sep 17 00:00:00 2001 From: Guido van der Hart Date: Wed, 4 Sep 2024 11:45:29 +0200 Subject: [PATCH 07/11] feat(VM): Improve robustness of vm and bastion reconcilers --- api/v1beta1/types.go | 31 +- cloud/scope/cluster.go | 20 +- cloud/scope/machine.go | 5 + .../services/compute/mock_compute/vm_mock.go | 24 +- cloud/services/compute/vm.go | 32 +- ...tructure.cluster.x-k8s.io_oscclusters.yaml | 4 +- ....cluster.x-k8s.io_oscclustertemplates.yaml | 8 +- ...tructure.cluster.x-k8s.io_oscmachines.yaml | 2 +- ....cluster.x-k8s.io_oscmachinetemplates.yaml | 6 +- config/rbac/role.yaml | 68 +--- controllers/osccluster_bastion_controller.go | 118 ++++--- ...osccluster_bastion_controller_unit_test.go | 134 -------- controllers/osccluster_controller.go | 9 +- controllers/oscmachine_controller.go | 13 +- controllers/oscmachine_vm_controller.go | 324 ++++++++++-------- .../oscmachine_vm_controller_unit_test.go | 64 ---- .../src/topics/get-started-with-clusterctl.md | 19 - 17 files changed, 302 insertions(+), 579 deletions(-) diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index b9039439a..0f750d3fe 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -378,22 +378,21 @@ type OscVm struct { } type OscBastion struct { - Name string `json:"name,omitempty"` - ImageId string `json:"imageId,omitempty"` - ImageName string `json:"imageName,omitempty"` - KeypairName string `json:"keypairName,omitempty"` - VmType string `json:"vmType,omitempty"` - DeviceName string `json:"deviceName,omitempty"` - SubnetName string `json:"subnetName,omitempty"` - RootDisk OscRootDisk `json:"rootDisk,omitempty"` - PublicIpName string `json:"publicIpName,omitempty"` - SubregionName string `json:"subregionName,omitempty"` - PrivateIps []OscPrivateIpElement `json:"privateIps,omitempty"` - SecurityGroupNames []OscSecurityGroupElement `json:"securityGroupNames,omitempty"` - ResourceId string `json:"resourceId,omitempty"` - ClusterName string `json:"clusterName,omitempty"` - Enable bool `json:"enable,omitempty"` - PublicIpNameAfterBastion bool `json:"publicIpNameAfterBastion,omitempty"` + Name string `json:"name,omitempty"` + ImageId string `json:"imageId,omitempty"` + ImageName string `json:"imageName,omitempty"` + KeypairName string `json:"keypairName,omitempty"` + VmType string `json:"vmType,omitempty"` + DeviceName string `json:"deviceName,omitempty"` + SubnetName string `json:"subnetName,omitempty"` + RootDisk OscRootDisk `json:"rootDisk,omitempty"` + PublicIpName string `json:"publicIpName,omitempty"` + SubregionName string `json:"subregionName,omitempty"` + PrivateIps []OscPrivateIpElement `json:"privateIps,omitempty"` + SecurityGroupNames []OscSecurityGroupElement `json:"securityGroupNames,omitempty"` + ResourceId string `json:"resourceId,omitempty"` + ClusterName string `json:"clusterName,omitempty"` + Enable bool `json:"enable,omitempty"` } type OscRootDisk struct { diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 20cd115d9..095ed224a 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -190,16 +190,6 @@ func (s *ClusterScope) SetExtraSecurityGroupRule(extraSecurityGroupRule bool) { s.OscCluster.Spec.Network.ExtraSecurityGroupRule = extraSecurityGroupRule } -// GetPublicIpNameAfterBastion return publicIpNameAfterBastion -func (s *ClusterScope) GetPublicIpNameAfterBastion() bool { - return s.OscCluster.Spec.Network.Bastion.PublicIpNameAfterBastion -} - -// SetPublicIpNameAfterBastion set the publicIpNameAfterBastion -func (s *ClusterScope) SetPublicIpNameAfterBastion(publicIpNameAfterBastion bool) { - s.OscCluster.Spec.Network.Bastion.PublicIpNameAfterBastion = publicIpNameAfterBastion -} - // GetNetwork return the network of the cluster func (s *ClusterScope) GetNetwork() *infrastructurev1beta1.OscNetwork { return &s.OscCluster.Spec.Network @@ -336,6 +326,11 @@ func (s *ClusterScope) GetControlPlaneEndpointPort() int32 { return s.OscCluster.Spec.ControlPlaneEndpoint.Port } +// GetReady get ready status +func (s *ClusterScope) GetReady() bool { + return s.OscCluster.Status.Ready +} + // SetNotReady set not ready status func (s *ClusterScope) SetNotReady() { s.OscCluster.Status.Ready = false @@ -386,6 +381,11 @@ func (s *ClusterScope) SetVmState(v infrastructurev1beta1.VmState) { s.OscCluster.Status.VmState = &v } +// SetVmState set vmstate +func (s *ClusterScope) GetVmState() *infrastructurev1beta1.VmState { + return s.OscCluster.Status.VmState +} + // PatchObject keep the cluster configuration and status func (s *ClusterScope) PatchObject() error { setConditions := []clusterv1.ConditionType{ diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index c07e88e5a..ebabb3e28 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -273,6 +273,11 @@ func (m *MachineScope) SetProviderID(subregionName string, vmId string) { m.OscMachine.Spec.ProviderID = pointer.StringPtr(pid) } +// SetVmID set the instanceID +func (m *MachineScope) SetVmID(vmId string) { + m.OscMachine.Spec.Node.Vm.ResourceId = vmId +} + // GetVmState return the vmState func (m *MachineScope) GetVmState() *infrastructurev1beta1.VmState { return m.OscMachine.Status.VmState diff --git a/cloud/services/compute/mock_compute/vm_mock.go b/cloud/services/compute/mock_compute/vm_mock.go index b5452422f..4a0819b02 100644 --- a/cloud/services/compute/mock_compute/vm_mock.go +++ b/cloud/services/compute/mock_compute/vm_mock.go @@ -6,7 +6,6 @@ package mock_compute import ( reflect "reflect" - time "time" gomock "github.com/golang/mock/gomock" v1beta1 "github.com/outscale-dev/cluster-api-provider-outscale.git/api/v1beta1" @@ -52,20 +51,6 @@ func (mr *MockOscVmInterfaceMockRecorder) AddCcmTag(clusterName, hostname, vmId return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddCcmTag", reflect.TypeOf((*MockOscVmInterface)(nil).AddCcmTag), clusterName, hostname, vmId) } -// CheckVmState mocks base method. -func (m *MockOscVmInterface) CheckVmState(clockInsideLoop, clockLoop time.Duration, state, vmId string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckVmState", clockInsideLoop, clockLoop, state, vmId) - ret0, _ := ret[0].(error) - return ret0 -} - -// CheckVmState indicates an expected call of CheckVmState. -func (mr *MockOscVmInterfaceMockRecorder) CheckVmState(clockInsideLoop, clockLoop, state, vmId interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckVmState", reflect.TypeOf((*MockOscVmInterface)(nil).CheckVmState), clockInsideLoop, clockLoop, state, vmId) -} - // CreateVm mocks base method. func (m *MockOscVmInterface) CreateVm(machineScope *scope.MachineScope, spec *v1beta1.OscVm, subnetId string, securityGroupIds, privateIps []string, vmName string, tags map[string]string) (*osc.Vm, error) { m.ctrl.T.Helper() @@ -125,6 +110,15 @@ func (mr *MockOscVmInterfaceMockRecorder) GetCapacity(tagKey, tagValue, vmType i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCapacity", reflect.TypeOf((*MockOscVmInterface)(nil).GetCapacity), tagKey, tagValue, vmType) } +// GetVmListFromTag mocks base method. +func (m *MockOscVmInterface) GetVmListFromTag(tagKey string, tagValue string) ([]osc.Vm, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetVmListFromTag", tagKey, tagValue) + ret0, _ := ret[0].([]osc.Vm) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + // GetVm mocks base method. func (m *MockOscVmInterface) GetVm(vmId string) (*osc.Vm, error) { m.ctrl.T.Helper() diff --git a/cloud/services/compute/vm.go b/cloud/services/compute/vm.go index 61f3b829f..53ab1bdae 100644 --- a/cloud/services/compute/vm.go +++ b/cloud/services/compute/vm.go @@ -23,11 +23,9 @@ import ( "net" "strconv" "strings" - "time" _nethttp "net/http" - "github.com/benbjohnson/clock" infrastructurev1beta1 "github.com/outscale-dev/cluster-api-provider-outscale.git/api/v1beta1" "github.com/outscale-dev/cluster-api-provider-outscale.git/cloud/scope" tag "github.com/outscale-dev/cluster-api-provider-outscale.git/cloud/tag" @@ -45,8 +43,8 @@ type OscVmInterface interface { CreateVmUserData(userData string, spec *infrastructurev1beta1.OscBastion, subnetId string, securityGroupIds []string, privateIps []string, vmName string, imageId string) (*osc.Vm, error) DeleteVm(vmId string) error GetVm(vmId string) (*osc.Vm, error) + GetVmListFromTag(tagKey string, tagName string) ([]osc.Vm, error) GetVmState(vmId string) (string, error) - CheckVmState(clockInsideLoop time.Duration, clockLoop time.Duration, state string, vmId string) error AddCcmTag(clusterName string, hostname string, vmId string) error GetCapacity(tagKey string, tagValue string, vmType string) (corev1.ResourceList, error) } @@ -413,32 +411,6 @@ func (s *Service) GetCapacity(tagKey string, tagValue string, vmType string) (co return capacity, nil } -// CheckVmState check the vm state -func (s *Service) CheckVmState(clockInsideLoop time.Duration, clockLoop time.Duration, state string, vmId string) error { - clock_time := clock.New() - currentTimeout := clock_time.Now().Add(time.Second * clockLoop) - var getVmState = false - for !getVmState { - time.Sleep(clockInsideLoop * time.Second) - vm, err := s.GetVm(vmId) - if err != nil { - return err - } - vmState, ok := vm.GetStateOk() - if !ok { - return errors.New("Can not get vm state") - } - if *vmState == state { - break - } - - if clock_time.Now().After(currentTimeout) { - return errors.New("Vm still not running") - } - } - return nil -} - // GetVmState return vm state func (s *Service) GetVmState(vmId string) (string, error) { vm, err := s.GetVm(vmId) @@ -447,7 +419,7 @@ func (s *Service) GetVmState(vmId string) (string, error) { } vmState, ok := vm.GetStateOk() if !ok { - return "", errors.New("Can not get vm state") + return "", errors.New("cannot get vm state") } return *vmState, nil } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclusters.yaml index f3816e0f4..08057636f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclusters.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.2 name: oscclusters.infrastructure.cluster.x-k8s.io spec: group: infrastructure.cluster.x-k8s.io @@ -85,8 +85,6 @@ spec: type: array publicIpName: type: string - publicIpNameAfterBastion: - type: boolean resourceId: type: string rootDisk: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclustertemplates.yaml index b8c2140b0..e11b7b0f5 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscclustertemplates.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.2 name: oscclustertemplates.infrastructure.cluster.x-k8s.io spec: group: infrastructure.cluster.x-k8s.io @@ -49,26 +49,22 @@ spec: ObjectMeta is metadata that all persisted resources must have, which includes all objects users must create. This is a copy of customizable fields from metav1.ObjectMeta. - ObjectMeta is embedded in `Machine.Spec`, `MachineDeployment.Template` and `MachineSet.Template`, which are not top-level Kubernetes objects. Given that metav1.ObjectMeta has lots of special cases and read-only fields which end up in the generated CRD validation, having it as a subset simplifies the API and some issues that can impact user experience. - During the [upgrade to controller-tools@v2](https://github.com/kubernetes-sigs/cluster-api/pull/1054) for v1alpha2, we noticed a failure would occur running Cluster API test suite against the new CRDs, specifically `spec.metadata.creationTimestamp in body must be of type string: "null"`. The investigation showed that `controller-tools@v2` behaves differently than its previous version when handling types from [metav1](k8s.io/apimachinery/pkg/apis/meta/v1) package. - In more details, we found that embedded (non-top level) types that embedded `metav1.ObjectMeta` had validation properties, including for `creationTimestamp` (metav1.Time). The `metav1.Time` type specifies a custom json marshaller that, when IsZero() is true, returns `null` which breaks validation because the field isn't marked as nullable. - In future versions, controller-tools@v2 might allow overriding the type and validation for embedded types. When that happens, this hack should be revisited. properties: @@ -141,8 +137,6 @@ spec: type: array publicIpName: type: string - publicIpNameAfterBastion: - type: boolean resourceId: type: string rootDisk: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachines.yaml index cfabece85..ce0c58800 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachines.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.2 name: oscmachines.infrastructure.cluster.x-k8s.io spec: group: infrastructure.cluster.x-k8s.io diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachinetemplates.yaml index 4c71d4260..ed321481d 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_oscmachinetemplates.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.2 name: oscmachinetemplates.infrastructure.cluster.x-k8s.io spec: group: infrastructure.cluster.x-k8s.io @@ -50,26 +50,22 @@ spec: ObjectMeta is metadata that all persisted resources must have, which includes all objects users must create. This is a copy of customizable fields from metav1.ObjectMeta. - ObjectMeta is embedded in `Machine.Spec`, `MachineDeployment.Template` and `MachineSet.Template`, which are not top-level Kubernetes objects. Given that metav1.ObjectMeta has lots of special cases and read-only fields which end up in the generated CRD validation, having it as a subset simplifies the API and some issues that can impact user experience. - During the [upgrade to controller-tools@v2](https://github.com/kubernetes-sigs/cluster-api/pull/1054) for v1alpha2, we noticed a failure would occur running Cluster API test suite against the new CRDs, specifically `spec.metadata.creationTimestamp in body must be of type string: "null"`. The investigation showed that `controller-tools@v2` behaves differently than its previous version when handling types from [metav1](k8s.io/apimachinery/pkg/apis/meta/v1) package. - In more details, we found that embedded (non-top level) types that embedded `metav1.ObjectMeta` had validation properties, including for `creationTimestamp` (metav1.Time). The `metav1.Time` type specifies a custom json marshaller that, when IsZero() is true, returns `null` which breaks validation because the field isn't marked as nullable. - In future versions, controller-tools@v2 might allow overriding the type and validation for embedded types. When that happens, this hack should be revisited. properties: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4b210e48e..0d134a7ad 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -27,29 +27,8 @@ rules: - cluster.x-k8s.io resources: - clusters - verbs: - - get - - list - - watch -- apiGroups: - - cluster.x-k8s.io - resources: - clusters/status - verbs: - - get - - list - - watch -- apiGroups: - - cluster.x-k8s.io - resources: - machines - verbs: - - get - - list - - watch -- apiGroups: - - cluster.x-k8s.io - resources: - machines/status verbs: - get @@ -59,32 +38,8 @@ rules: - infrastructure.cluster.x-k8s.io resources: - oscclusters - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - infrastructure.cluster.x-k8s.io - resources: - - oscclusters/finalizers - verbs: - - update -- apiGroups: - - infrastructure.cluster.x-k8s.io - resources: - - oscclusters/status - verbs: - - get - - patch - - update -- apiGroups: - - infrastructure.cluster.x-k8s.io - resources: - oscmachines + - oscmachinetemplates verbs: - create - delete @@ -96,32 +51,15 @@ rules: - apiGroups: - infrastructure.cluster.x-k8s.io resources: + - oscclusters/finalizers - oscmachines/finalizers verbs: - update - apiGroups: - infrastructure.cluster.x-k8s.io resources: + - oscclusters/status - oscmachines/status - verbs: - - get - - patch - - update -- apiGroups: - - infrastructure.cluster.x-k8s.io - resources: - - oscmachinetemplates - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - infrastructure.cluster.x-k8s.io - resources: - oscmachinetemplates/status verbs: - get diff --git a/controllers/osccluster_bastion_controller.go b/controllers/osccluster_bastion_controller.go index 26e70916f..925746b11 100644 --- a/controllers/osccluster_bastion_controller.go +++ b/controllers/osccluster_bastion_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "time" "fmt" @@ -251,86 +252,85 @@ func reconcileBastion(ctx context.Context, clusterScope *scope.ClusterScope, vmS } securityGroupIds = append(securityGroupIds, securityGroupId) } - var bastion *osc.Vm - var vmID string + var vm *osc.Vm + var vmId string if len(bastionRef.ResourceMap) == 0 { bastionRef.ResourceMap = make(map[string]string) } - publicIpNameAfterBastion := clusterScope.GetPublicIpNameAfterBastion() clusterScope.V(4).Info("Get ResourceId", "resourceId", bastionSpec.ResourceId) - tagKey := "Name" - tagValue := bastionName - tag, err := tagSvc.ReadTag(tagKey, tagValue) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get tag for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) - } - if bastionSpec.ResourceId != "" { - bastionRef.ResourceMap[bastionName] = bastionSpec.ResourceId - vmId := bastionSpec.ResourceId - clusterScope.V(4).Info("Get vmId", "bastion", bastionRef.ResourceMap) - bastion, err = vmSvc.GetVm(vmId) + bastionState := clusterScope.GetVmState() + + if bastionState == nil { + vms, err := vmSvc.GetVmListFromTag("Name", bastionName) if err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, fmt.Errorf("%w Could not list vms for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) } - clusterScope.V(2).Info("Get VmState") - vmState, err := vmSvc.GetVmState(vmId) - if err != nil { - clusterScope.SetVmState(infrastructurev1beta1.VmState("unknown")) - return reconcile.Result{}, fmt.Errorf("%w Can not get bastion %s state for OscCluster %s/%s", err, vmId, clusterScope.GetNamespace(), clusterScope.GetName()) + if len(vms) > 0 { + if bastionSpec.ResourceId != "" { + clusterScope.SetVmState(infrastructurev1beta1.VmStatePending) + bastionRef.ResourceMap[bastionName] = bastionSpec.ResourceId + return reconcile.Result{RequeueAfter: 30 * time.Second}, nil + } + return reconcile.Result{}, fmt.Errorf("%w Bastion Vm with Name %s already exists for OscCluster %s/%s", err, bastionName, clusterScope.GetNamespace(), clusterScope.GetName()) } - clusterScope.SetVmState(infrastructurev1beta1.VmState(vmState)) - clusterScope.V(4).Info("Get bastion state", "vmState", vmState) - } - if (bastion == nil && tag == nil) || (bastionSpec.ResourceId == "" && tag == nil) { + clusterScope.V(4).Info("Create the desired bastion", "bastionName", bastionName) keypairName := bastionSpec.KeypairName clusterScope.V(4).Info("Get keypairName", "keypairName", keypairName) vmType := bastionSpec.VmType clusterScope.V(4).Info("Get vmType", "vmType", vmType) - vm, err := vmSvc.CreateVmUserData("", bastionSpec, subnetId, securityGroupIds, privateIps, bastionName, imageId) + vm, err = vmSvc.CreateVmUserData("", bastionSpec, subnetId, securityGroupIds, privateIps, bastionName, imageId) if err != nil { return reconcile.Result{}, fmt.Errorf("%w Can not create bastion for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) } - vmID = vm.GetVmId() - err = vmSvc.CheckVmState(5, 120, "running", vmID) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s running for OscCluster %s/%s", err, vmID, clusterScope.GetNamespace(), clusterScope.GetName()) - } - clusterScope.V(4).Info("Bastion is running", "vmID", vmID) - clusterScope.SetVmState(infrastructurev1beta1.VmState("pending")) - if bastionSpec.PublicIpName != "" { - linkPublicIpId, err := publicIpSvc.LinkPublicIp(publicIpId, vmID) + vmId = vm.GetVmId() + clusterScope.SetVmState(infrastructurev1beta1.VmStatePending) + bastionState = &infrastructurev1beta1.VmStatePending + bastionRef.ResourceMap[bastionName] = vmId + bastionSpec.ResourceId = vmId + clusterScope.V(4).Info("Bastion Created", "bastionId", vmId) + } + + if bastionState != nil { + if *bastionState != infrastructurev1beta1.VmStateRunning { + vmId := bastionSpec.ResourceId + clusterScope.V(4).Info("Get vmId", "vmId", vmId) + _, err = vmSvc.GetVm(vmId) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not link publicIp %s with %s for OscCluster %s/%s", err, publicIpId, vmID, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, err } - err = vmSvc.CheckVmState(5, 120, "running", vmID) + clusterScope.V(2).Info("Get currentVmState") + currentVmState, err := vmSvc.GetVmState(vmId) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s running for OscCluster %s/%s", err, vmID, clusterScope.GetNamespace(), clusterScope.GetName()) + clusterScope.SetVmState(infrastructurev1beta1.VmState("unknown")) + return reconcile.Result{}, fmt.Errorf("%w Can not get bastion %s state for OscCluster %s/%s", err, vmId, clusterScope.GetNamespace(), clusterScope.GetName()) } - clusterScope.V(4).Info("Get bastionPublicIpName", "bastionPublicIpName", bastionPublicIpName) - linkPublicIpRef.ResourceMap[bastionPublicIpName] = linkPublicIpId + clusterScope.SetVmState(infrastructurev1beta1.VmState(currentVmState)) + clusterScope.V(4).Info("Bastion state", "vmState", currentVmState) + if infrastructurev1beta1.VmState(currentVmState) != infrastructurev1beta1.VmStateRunning { + clusterScope.V(4).Info("Bastion vm is not yet running", "vmId", vmId) + return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("bastion %s is not yet running for OscCluster %s/%s", vmId, clusterScope.GetNamespace(), clusterScope.GetName()) + } + bastionState = &infrastructurev1beta1.VmStateRunning + clusterScope.V(4).Info("Bastion is running", "vmId", vmId) } - bastionRef.ResourceMap[bastionName] = vmID - bastionSpec.ResourceId = vmID - } - if publicIpNameAfterBastion && bastionSpec.PublicIpName != "" && bastionSpec.ResourceId != "" { - linkPublicIpId, err := publicIpSvc.LinkPublicIp(publicIpId, bastionSpec.ResourceId) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not link publicIp %s with %s for OscCluster %s/%s", err, publicIpId, vmID, clusterScope.GetNamespace(), clusterScope.GetName()) - } - err = vmSvc.CheckVmState(5, 120, "running", bastionSpec.ResourceId) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s running for OscCluster %s/%s", err, vmID, clusterScope.GetNamespace(), clusterScope.GetName()) - } - bastionPublicIpName := bastionSpec.PublicIpName + "-" + clusterScope.GetUID() - clusterScope.V(4).Info("Get bastionPublicIpName", "bastionPublicIpName", bastionPublicIpName) - linkPublicIpRef.ResourceMap[bastionPublicIpName] = linkPublicIpId - bastionRef.ResourceMap[bastionName] = bastionSpec.ResourceId - } + if *bastionState == infrastructurev1beta1.VmStateRunning { + vmId := bastionSpec.ResourceId + if bastionSpec.PublicIpName != "" && linkPublicIpRef.ResourceMap[bastionPublicIpName] == "" { + linkPublicIpId, err := publicIpSvc.LinkPublicIp(publicIpId, vmId) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not link publicIp %s with %s for OscCluster %s/%s", err, publicIpId, vmId, clusterScope.GetNamespace(), clusterScope.GetName()) + } + clusterScope.V(4).Info("Get bastionPublicIpName", "bastionPublicIpName", bastionPublicIpName) + linkPublicIpRef.ResourceMap[bastionPublicIpName] = linkPublicIpId + } + } + } + clusterScope.V(4).Info("Bastion is reconciled") return reconcile.Result{}, nil } @@ -340,7 +340,7 @@ func reconcileDeleteBastion(ctx context.Context, clusterScope *scope.ClusterScop bastionSpec := clusterScope.GetBastion() bastionSpec.SetDefaultValue() vmId := bastionSpec.ResourceId - clusterScope.V(4).Info("Get VmID", "vmId", vmId) + clusterScope.V(4).Info("Get vmId", "vmId", vmId) bastionName := bastionSpec.Name if bastionSpec.ResourceId == "" { clusterScope.V(2).Info("The desired bastion is currently destroyed", "bastionName", bastionName) @@ -351,15 +351,13 @@ func reconcileDeleteBastion(ctx context.Context, clusterScope *scope.ClusterScop return reconcile.Result{}, err } - var securityGroupIds []string bastionSecurityGroups := clusterScope.GetBastionSecurityGroups() for _, bastionSecurityGroup := range *bastionSecurityGroups { securityGroupName := bastionSecurityGroup.Name + "-" + clusterScope.GetUID() - securityGroupId, err := getSecurityGroupResourceId(securityGroupName, clusterScope) + _, err := getSecurityGroupResourceId(securityGroupName, clusterScope) if err != nil { return reconcile.Result{}, err } - securityGroupIds = append(securityGroupIds, securityGroupId) } if bastion == nil { clusterScope.V(2).Info("The desired bastion does not exist anymore", "bastionName", bastionName) diff --git a/controllers/osccluster_bastion_controller_unit_test.go b/controllers/osccluster_bastion_controller_unit_test.go index 4d95b3b5f..7fe04d12b 100644 --- a/controllers/osccluster_bastion_controller_unit_test.go +++ b/controllers/osccluster_bastion_controller_unit_test.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "testing" - "time" "github.com/golang/mock/gomock" infrastructurev1beta1 "github.com/outscale-dev/cluster-api-provider-outscale.git/api/v1beta1" @@ -180,85 +179,6 @@ var ( }, } - defaultPublicIpNameAfterBastionReconcile = infrastructurev1beta1.OscClusterSpec{ - Network: infrastructurev1beta1.OscNetwork{ - Net: infrastructurev1beta1.OscNet{ - Name: "test-net", - IpRange: "10.0.0.0/16", - ClusterName: "test-cluster", - ResourceId: "vpc-test-net-uid", - }, - Subnets: []*infrastructurev1beta1.OscSubnet{ - { - Name: "test-subnet", - IpSubnetRange: "10.0.0.0/24", - SubregionName: "eu-west-2a", - ResourceId: "subnet-test-subnet-uid", - }, - }, - SecurityGroups: []*infrastructurev1beta1.OscSecurityGroup{ - { - Name: "test-securitygroup", - Description: "test securitygroup", - ResourceId: "sg-test-securitygroup-uid", - SecurityGroupRules: []infrastructurev1beta1.OscSecurityGroupRule{ - { - Name: "test-securitygrouprule", - Flow: "Inbound", - IpProtocol: "tcp", - IpRange: "0.0.0.0/0", - FromPortRange: 6443, - ToPortRange: 6443, - }, - }, - }, - }, - LoadBalancer: infrastructurev1beta1.OscLoadBalancer{ - LoadBalancerName: "test-loadbalancer", - LoadBalancerType: "internet-facing", - SubnetName: "test-subnet", - SecurityGroupName: "test-securitygroup", - }, - PublicIps: []*infrastructurev1beta1.OscPublicIp{ - { - Name: "test-publicip", - ResourceId: "test-publicip-uid", - }, - }, - Bastion: infrastructurev1beta1.OscBastion{ - Enable: true, - ClusterName: "test-cluster", - Name: "test-bastion", - ImageId: "ami-00000000", - DeviceName: "/dev/xvdb", - KeypairName: "rke", - RootDisk: infrastructurev1beta1.OscRootDisk{ - - RootDiskSize: 30, - RootDiskIops: 1500, - RootDiskType: "io1", - }, - SubregionName: "eu-west-2a", - SubnetName: "test-subnet", - VmType: "tinav3.c2r4p2", - ResourceId: "i-test-bastion-uid", - PublicIpName: "test-publicip", - PublicIpNameAfterBastion: true, - SecurityGroupNames: []infrastructurev1beta1.OscSecurityGroupElement{ - { - Name: "test-securitygroup", - }, - }, - PrivateIps: []infrastructurev1beta1.OscPrivateIpElement{ - { - Name: "test-privateip", - PrivateIp: "10.0.0.17", - }, - }, - }, - }, - } - defaultBastionReconcile = infrastructurev1beta1.OscClusterSpec{ Network: infrastructurev1beta1.OscNetwork{ Net: infrastructurev1beta1.OscNet{ @@ -1835,7 +1755,6 @@ func TestReconcileBastion(t *testing.T) { clusterScope, ctx, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface := SetupWithBastionMock(t, btc.name, btc.clusterSpec) bastionName := btc.clusterSpec.Network.Bastion.Name + "-uid" vmId := "i-" + bastionName - vmState := "running" subnetName := btc.clusterSpec.Network.Bastion.SubnetName + "-uid" subnetId := "subnet-" + subnetName @@ -1877,8 +1796,6 @@ func TestReconcileBastion(t *testing.T) { } bastionSpec := btc.clusterSpec.Network.Bastion - var clockInsideLoop time.Duration = 5 - var firstClockLoop time.Duration = 120 createVms := osc.CreateVmsResponse{ Vms: &[]osc.Vm{ { @@ -1918,12 +1835,6 @@ func TestReconcileBastion(t *testing.T) { CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). Return(nil, btc.expCreateVmErr) } - if btc.expCheckVmStateBootFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(btc.expCheckVmStateBootErr) - } if btc.expLinkPublicIpFound { mockOscPublicIpInterface. @@ -1931,12 +1842,6 @@ func TestReconcileBastion(t *testing.T) { LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). Return(*linkPublicIp.LinkPublicIpId, btc.expLinkPublicIpErr) } - if btc.expCheckVmStatePublicIpFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(btc.expCheckVmStatePublicIpErr) - } reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) if err != nil { @@ -2134,7 +2039,6 @@ func TestReconcileLinkBastion(t *testing.T) { clusterScope, ctx, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface := SetupWithBastionMock(t, btc.name, btc.clusterSpec) bastionName := btc.clusterSpec.Network.Bastion.Name + "-uid" vmId := "i-" + bastionName - vmState := "running" subnetName := btc.clusterSpec.Network.Bastion.SubnetName + "-uid" subnetId := "subnet-" + subnetName @@ -2177,9 +2081,6 @@ func TestReconcileLinkBastion(t *testing.T) { } bastionSpec := btc.clusterSpec.Network.Bastion - var clockInsideLoop time.Duration = 5 - var clockLoop time.Duration = 120 - var firstClockLoop time.Duration = 120 createVms := osc.CreateVmsResponse{ Vms: &[]osc.Vm{ { @@ -2208,12 +2109,6 @@ func TestReconcileLinkBastion(t *testing.T) { } bastion := &createVm[0] - if btc.expCheckVmStateBootFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(btc.expCheckVmStateBootErr) - } if btc.expCreateVmFound { mockOscVmInterface. EXPECT(). @@ -2232,12 +2127,6 @@ func TestReconcileLinkBastion(t *testing.T) { LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). Return(*linkPublicIp.LinkPublicIpId, btc.expLinkPublicIpErr) } - if btc.expCheckVmStatePublicIpFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(btc.expCheckVmStatePublicIpErr) - } reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) if err != nil { @@ -2283,21 +2172,6 @@ func TestReconcileBastionGet(t *testing.T) { expLinkPublicIpErr: nil, expReconcileBastionErr: nil, }, - { - name: "get bastion with publicIpNameAfterBastion", - clusterSpec: defaultPublicIpNameAfterBastionReconcile, - expLinkPublicIpFound: true, - expGetVmFound: true, - expGetVmStateFound: true, - expTagFound: false, - expCheckVmStatePublicIpFound: true, - expGetVmErr: nil, - expGetVmStateErr: nil, - expReadTagErr: nil, - expCheckVmStatePublicIpErr: nil, - expLinkPublicIpErr: nil, - expReconcileBastionErr: nil, - }, { name: "failed to get bastion", clusterSpec: defaultBastionReconcile, @@ -2373,8 +2247,6 @@ func TestReconcileBastionGet(t *testing.T) { securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId securityGroupIds = append(securityGroupIds, securityGroupId) } - var clockInsideLoop time.Duration = 5 - var clockLoop time.Duration = 120 readVms := osc.ReadVmsResponse{ Vms: &[]osc.Vm{ { @@ -2424,12 +2296,6 @@ func TestReconcileBastionGet(t *testing.T) { LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). Return(*linkPublicIp.LinkPublicIpId, btc.expLinkPublicIpErr) } - if btc.expCheckVmStatePublicIpFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(btc.expCheckVmStatePublicIpErr) - } reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) if err != nil { diff --git a/controllers/osccluster_controller.go b/controllers/osccluster_controller.go index 1b2035dfb..b87d69ff7 100644 --- a/controllers/osccluster_controller.go +++ b/controllers/osccluster_controller.go @@ -322,8 +322,7 @@ func (r *OscClusterReconciler) reconcile(ctx context.Context, clusterScope *scop return reconcile.Result{}, fmt.Errorf("%w Can not create bastion %s for OscCluster %s/%s", err, bastionName, clusterScope.GetNamespace(), clusterScope.GetName()) } } - clusterScope.V(2).Info("Set OscCluster status to not ready") - clusterScope.SetNotReady() + // Reconcile each element of the cluster netSvc := r.getNetSvc(ctx, *clusterScope) tagSvc := r.getTagSvc(ctx, *clusterScope) @@ -411,16 +410,20 @@ func (r *OscClusterReconciler) reconcile(ctx context.Context, clusterScope *scop conditions.MarkFalse(osccluster, infrastructurev1beta1.LoadBalancerReadyCondition, infrastructurev1beta1.LoadBalancerFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) return reconcileLoadBalancer, err } + if clusterScope.OscCluster.Spec.Network.Bastion.Enable { + clusterScope.V(4).Info("Reconciling bastion Vm") vmSvc := r.getVmSvc(ctx, *clusterScope) imageSvc := r.getImageSvc(ctx, *clusterScope) reconcileBastion, err := reconcileBastion(ctx, clusterScope, vmSvc, publicIpSvc, securityGroupSvc, imageSvc, tagSvc) if err != nil { clusterScope.Error(err, "failed to reconcile bastion") - conditions.MarkFalse(osccluster, infrastructurev1beta1.VmReadyCondition, infrastructurev1beta1.VmNotReadyReason, clusterv1.ConditionSeverityWarning, err.Error()) + conditions.MarkFalse(osccluster, infrastructurev1beta1.VmReadyCondition, infrastructurev1beta1.VmNotReadyReason, clusterv1.ConditionSeverityWarning, "%s", err.Error()) return reconcileBastion, err } } + conditions.MarkTrue(osccluster, infrastructurev1beta1.VmReadyCondition) + clusterScope.V(2).Info("Set OscCluster status to ready") clusterScope.SetReady() return reconcile.Result{}, nil diff --git a/controllers/oscmachine_controller.go b/controllers/oscmachine_controller.go index 22abb5c8d..c7344771d 100644 --- a/controllers/oscmachine_controller.go +++ b/controllers/oscmachine_controller.go @@ -200,8 +200,6 @@ func (r *OscMachineReconciler) reconcile(ctx context.Context, machineScope *scop controllerutil.AddFinalizer(oscmachine, "oscmachine.infrastructure.cluster.x-k8s.io") - machineScope.V(2).Info("Set OscMachine status to not ready") - machineScope.SetNotReady() if !machineScope.Cluster.Status.InfrastructureReady { machineScope.V(2).Info("Cluster infrastructure is not ready yet") conditions.MarkFalse(oscmachine, infrastructurev1beta1.VmReadyCondition, infrastructurev1beta1.WaitingForClusterInfrastructureReason, clusterv1.ConditionSeverityInfo, "") @@ -308,6 +306,7 @@ func (r *OscMachineReconciler) reconcile(ctx context.Context, machineScope *scop return reconcileVolume, err } } + conditions.MarkTrue(oscmachine, infrastructurev1beta1.VolumeReadyCondition) keypairSvc := r.getKeyPairSvc(ctx, *clusterScope) reconcileKeypair, err := reconcileKeypair(ctx, machineScope, keypairSvc) @@ -320,20 +319,16 @@ func (r *OscMachineReconciler) reconcile(ctx context.Context, machineScope *scop vmSvc := r.getVmSvc(ctx, *clusterScope) loadBalancerSvc := r.getLoadBalancerSvc(ctx, *clusterScope) securityGroupSvc := r.getSecurityGroupSvc(ctx, *clusterScope) + + machineScope.V(4).Info("Reconciling Vm") reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, vmSvc, volumeSvc, publicIpSvc, loadBalancerSvc, securityGroupSvc, tagSvc) if err != nil { machineScope.Error(err, "failed to reconcile vm") - conditions.MarkFalse(oscmachine, infrastructurev1beta1.VmReadyCondition, infrastructurev1beta1.VmNotReadyReason, clusterv1.ConditionSeverityWarning, err.Error()) + conditions.MarkFalse(oscmachine, infrastructurev1beta1.VmReadyCondition, infrastructurev1beta1.VmNotReadyReason, clusterv1.ConditionSeverityWarning, "%s", err.Error()) return reconcileVm, err } - conditions.MarkTrue(oscmachine, infrastructurev1beta1.VolumeReadyCondition) vmState := machineScope.GetVmState() - if vmState == nil { - machineScope.V(2).Info("VmState is not yet availablle") - return ctrl.Result{}, nil - } - switch *vmState { case infrastructurev1beta1.VmStatePending: machineScope.SetNotReady() diff --git a/controllers/oscmachine_vm_controller.go b/controllers/oscmachine_vm_controller.go index 24519b834..b01d5560d 100644 --- a/controllers/oscmachine_vm_controller.go +++ b/controllers/oscmachine_vm_controller.go @@ -32,7 +32,6 @@ import ( osc "github.com/outscale/osc-sdk-go/v2" corev1 "k8s.io/api/core/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -382,6 +381,13 @@ func reconcileVm(ctx context.Context, clusterScope *scope.ClusterScope, machineS } } + if vmSpec.KeypairName != "" { + _, err = getKeyPairResourceId(vmSpec.KeypairName, machineScope) + if err != nil { + return reconcile.Result{}, err + } + } + var securityGroupIds []string vmSecurityGroups := machineScope.GetVmSecurityGroups() for _, vmSecurityGroup := range *vmSecurityGroups { @@ -400,64 +406,31 @@ func reconcileVm(ctx context.Context, clusterScope *scope.ClusterScope, machineS vmVolumeDeviceName = vmSpec.VolumeDeviceName } var vm *osc.Vm - var vmID string + var vmId string if len(vmRef.ResourceMap) == 0 { vmRef.ResourceMap = make(map[string]string) } - tagKey := "Name" - tagValue := vmName - tag, err := tagSvc.ReadTag(tagKey, tagValue) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get tag for OscMachine %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) - } - machineScope.V(4).Info("Get ResourceId", "resourceId", vmSpec.ResourceId) - machineScope.V(4).Info("Get ResourceMap", "resourceMap", vmRef.ResourceMap) - if vmSpec.ResourceId != "" { - vmRef.ResourceMap[vmName] = vmSpec.ResourceId - vmId := vmSpec.ResourceId - machineScope.V(4).Info("Check if the desired vm exist", "vmName", vmName) - vm, err = vmSvc.GetVm(vmId) - if err != nil { - return reconcile.Result{}, err - } - clusterName := vmSpec.ClusterName + "-" + clusterScope.GetUID() - privateDnsName, ok := vm.GetPrivateDnsNameOk() - if !ok { - return reconcile.Result{}, fmt.Errorf("Can not found privateDnsName %s/%s", machineScope.GetNamespace(), machineScope.GetName()) - } - privateIp, ok := vm.GetPrivateIpOk() - if !ok { - return reconcile.Result{}, fmt.Errorf("Can not found privateIp %s/%s", machineScope.GetNamespace(), machineScope.GetName()) - } - addresses := []corev1.NodeAddress{} - addresses = append( - addresses, - corev1.NodeAddress{ - Type: corev1.NodeInternalIP, - Address: *privateIp, - }, - ) - // Expose Public IP if one is set - if publicIp, ok := vm.GetPublicIpOk(); ok { - addresses = append(addresses, corev1.NodeAddress{ - Type: corev1.NodeExternalIP, - Address: *publicIp, - }) - } - machineScope.SetAddresses(addresses) - err = vmSvc.AddCcmTag(clusterName, *privateDnsName, vmId) + vmState := machineScope.GetVmState() + + if vmState == nil { + vms, err := vmSvc.GetVmListFromTag("Name", vmName) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w can not add ccm tag %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) + return reconcile.Result{}, fmt.Errorf("%w Could not list vms for OscCluster %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) } - vmState, err := vmSvc.GetVmState(vmId) - if err != nil { - machineScope.SetVmState(infrastructurev1beta1.VmState("unknown")) - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s state for OscMachine %s/%s", err, vmId, machineScope.GetNamespace(), machineScope.GetName()) + if len(vms) > 0 { + if vmSpec.ResourceId != "" || vmRef.ResourceMap[vmName] != "" { // We should not get in this situation but we sometimes do (To be investigated) + machineScope.SetVmState(infrastructurev1beta1.VmStatePending) + if vmSpec.ResourceId != "" { + vmRef.ResourceMap[vmName] = vmSpec.ResourceId + } + if vmRef.ResourceMap[vmName] != "" { + machineScope.SetVmID(vmRef.ResourceMap[vmName]) + } + return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("%w Vm with Name %s is already created for OscCluster %s/%s", err, vmName, machineScope.GetNamespace(), machineScope.GetName()) + } + return reconcile.Result{}, fmt.Errorf("%w Vm with Name %s already exists for OscCluster %s/%s", err, vmName, machineScope.GetNamespace(), machineScope.GetName()) } - machineScope.SetVmState(infrastructurev1beta1.VmState(vmState)) - machineScope.V(4).Info("Get vm state", "vmState", vmState) - } - if (vm == nil && tag == nil) || (vmSpec.ResourceId == "" && tag == nil) { + machineScope.V(4).Info("Create the desired vm", "vmName", vmName) imageId := vmSpec.ImageId machineScope.V(4).Info("Info ImageId", "imageId", imageId) @@ -473,122 +446,198 @@ func reconcileVm(ctx context.Context, clusterScope *scope.ClusterScope, machineS return reconcile.Result{}, fmt.Errorf("%w Can not create vm for OscMachine %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) } - vmID = vm.GetVmId() - err = vmSvc.CheckVmState(20, 240, "running", vmID) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s running for OscMachine %s/%s", err, vmID, machineScope.GetNamespace(), machineScope.GetName()) - } - machineScope.V(4).Info("Vm is running", "vmId", vmID) - machineScope.SetVmState(infrastructurev1beta1.VmState("pending")) - if vmSpec.VolumeName != "" { - err = volumeSvc.CheckVolumeState(20, 240, "available", volumeId) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get volume %s available for OscMachine %s/%s", err, volumeId, machineScope.GetNamespace(), machineScope.GetName()) + vmId = vm.GetVmId() + machineScope.SetVmState(infrastructurev1beta1.VmStatePending) + vmState = &infrastructurev1beta1.VmStatePending + vmRef.ResourceMap[vmName] = vmId + machineScope.SetVmID(vmId) + subregionName := vmSpec.SubregionName + machineScope.SetProviderID(subregionName, vmId) + machineScope.V(4).Info("Vm created", "vmId", vmId) + } + + if vmState != nil { + if *vmState != infrastructurev1beta1.VmStateRunning { + vmId := vmSpec.ResourceId + if vmId == "" { // We should not get in this situation but we sometimes do (To be investigated) + vmId = vmRef.ResourceMap[vmName] + machineScope.SetVmID(vmId) + subregionName := vmSpec.SubregionName + machineScope.SetProviderID(subregionName, vmId) } - machineScope.V(4).Info("Volume is available", "volumeId", volumeId) - err = volumeSvc.LinkVolume(volumeId, vmID, vmVolumeDeviceName) + machineScope.V(4).Info("Get vmId", "vmId", vmId) + _, err = vmSvc.GetVm(vmId) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not link volume %s with vm %s for OscMachine %s/%s", err, volumeId, vmID, machineScope.GetNamespace(), machineScope.GetName()) + return reconcile.Result{}, err } - machineScope.V(4).Info("Volume is linked", "volumeId", volumeId) - err = volumeSvc.CheckVolumeState(20, 240, "in-use", volumeId) - machineScope.V(4).Info("Volume is in-use", "volumeId", volumeId) + machineScope.V(2).Info("Get currentVmState") + currentVmState, err := vmSvc.GetVmState(vmId) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get volume %s in use for OscMachine %s/%s", err, volumeId, machineScope.GetNamespace(), machineScope.GetName()) + machineScope.SetVmState(infrastructurev1beta1.VmState("unknown")) + return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s state for OscCluster %s/%s", err, vmId, machineScope.GetNamespace(), machineScope.GetName()) } - } - err = vmSvc.CheckVmState(20, 240, "running", vmID) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s running for OscMachine %s/%s", err, vmID, machineScope.GetNamespace(), machineScope.GetName()) - } - machineScope.V(4).Info("Vm is running again", "vmId", vmID) + machineScope.SetVmState(infrastructurev1beta1.VmState(currentVmState)) + machineScope.V(4).Info("Vm state", "vmState", currentVmState) - if vmSpec.PublicIpName != "" { - linkPublicIpId, err := publicIpSvc.LinkPublicIp(publicIpId, vmID) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not link publicIp %s with %s for OscCluster %s/%s", err, publicIpId, vmID, machineScope.GetNamespace(), machineScope.GetName()) + if infrastructurev1beta1.VmState(currentVmState) != infrastructurev1beta1.VmStateRunning { + machineScope.V(4).Info("Vm is not yet running", "vmId", vmId) + return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("vm %s is not yet running for OscCluster %s/%s", vmId, machineScope.GetNamespace(), machineScope.GetName()) } - machineScope.V(4).Info("Link public ip", "linkPublicIpId", linkPublicIpId) - linkPublicIpRef.ResourceMap[vmPublicIpName] = linkPublicIpId + vmState = &infrastructurev1beta1.VmStateRunning + machineScope.V(4).Info("Vm is running", "vmId", vmId) + } - err = vmSvc.CheckVmState(20, 240, "running", vmID) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get vm %s running for OscMachine %s/%s", err, vmID, machineScope.GetNamespace(), machineScope.GetName()) + if *vmState == infrastructurev1beta1.VmStateRunning { + vmId := vmSpec.ResourceId + if vmId == "" { // We should not get in this situation but we sometimes do (To be investigated) + vmId = vmRef.ResourceMap[vmName] + machineScope.SetVmID(vmId) + subregionName := vmSpec.SubregionName + machineScope.SetProviderID(subregionName, vmId) } - } - if vmSpec.LoadBalancerName != "" { - loadBalancerName := vmSpec.LoadBalancerName - vmIds := []string{vmID} - err := loadBalancerSvc.LinkLoadBalancerBackendMachines(vmIds, loadBalancerName) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not link vm %s with loadBalancerName %s for OscCluster %s/%s", err, loadBalancerName, vmID, machineScope.GetNamespace(), machineScope.GetName()) + if vmSpec.VolumeName != "" { + err = volumeSvc.CheckVolumeState(20, 240, "available", volumeId) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not get volume %s available for OscMachine %s/%s", err, volumeId, machineScope.GetNamespace(), machineScope.GetName()) + } + machineScope.V(4).Info("Volume is available", "volumeId", volumeId) + err = volumeSvc.LinkVolume(volumeId, vmId, vmVolumeDeviceName) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not link volume %s with vm %s for OscMachine %s/%s", err, volumeId, vmId, machineScope.GetNamespace(), machineScope.GetName()) + } + machineScope.V(4).Info("Volume is linked", "volumeId", volumeId) + err = volumeSvc.CheckVolumeState(20, 240, "in-use", volumeId) + machineScope.V(4).Info("Volume is in-use", "volumeId", volumeId) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not get volume %s in use for OscMachine %s/%s", err, volumeId, machineScope.GetNamespace(), machineScope.GetName()) + } } - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - loadBalancerSpec := clusterScope.GetLoadBalancer() - loadBalancerSpec.SetDefaultValue() - loadBalancerSecurityGroupName := loadBalancerSpec.SecurityGroupName - ipProtocol := strings.ToLower(loadBalancerSpec.Listener.BackendProtocol) - machineScope.V(4).Info("Get IpProtocol", "IpProtocol", ipProtocol) - fromPortRange := loadBalancerSpec.Listener.BackendPort - machineScope.V(4).Info("Get fromPortRange", "fromPortRange", fromPortRange) - toPortRange := loadBalancerSpec.Listener.BackendPort - machineScope.V(4).Info("Get ToPortRange", "ToPortRange", toPortRange) - loadBalancerSecurityGroupClusterScopeName := loadBalancerSecurityGroupName + "-" + clusterScope.GetUID() - associateSecurityGroupId := securityGroupsRef.ResourceMap[loadBalancerSecurityGroupClusterScopeName] - machineScope.V(4).Info("Get sg", "associateSecurityGroupId", associateSecurityGroupId) - securityGroupFromSecurityGroupOutboundRule, err := securityGroupSvc.GetSecurityGroupFromSecurityGroupRule(associateSecurityGroupId, "Outbound", ipProtocol, "", securityGroupIds[0], fromPortRange, toPortRange) - if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get outbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + + if vmSpec.PublicIpName != "" && linkPublicIpRef.ResourceMap[vmPublicIpName] == "" { + linkPublicIpId, err := publicIpSvc.LinkPublicIp(publicIpId, vmId) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not link publicIp %s with %s for OscCluster %s/%s", err, publicIpId, vmId, machineScope.GetNamespace(), machineScope.GetName()) + } + machineScope.V(4).Info("Link public ip", "linkPublicIpId", linkPublicIpId) + linkPublicIpRef.ResourceMap[vmPublicIpName] = linkPublicIpId } - if securityGroupFromSecurityGroupOutboundRule == nil { - _, err = securityGroupSvc.CreateSecurityGroupRule(associateSecurityGroupId, "Outbound", ipProtocol, "", securityGroupIds[0], fromPortRange, toPortRange) + if vmSpec.LoadBalancerName != "" { + loadBalancerName := vmSpec.LoadBalancerName + vmIds := []string{vmId} + err := loadBalancerSvc.LinkLoadBalancerBackendMachines(vmIds, loadBalancerName) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not link vm %s with loadBalancerName %s for OscCluster %s/%s", err, vmId, loadBalancerName, machineScope.GetNamespace(), machineScope.GetName()) + } + securityGroupsRef := clusterScope.GetSecurityGroupsRef() + loadBalancerSpec := clusterScope.GetLoadBalancer() + loadBalancerSpec.SetDefaultValue() + loadBalancerSecurityGroupName := loadBalancerSpec.SecurityGroupName + ipProtocol := strings.ToLower(loadBalancerSpec.Listener.BackendProtocol) + machineScope.V(4).Info("Get IpProtocol", "IpProtocol", ipProtocol) + fromPortRange := loadBalancerSpec.Listener.BackendPort + machineScope.V(4).Info("Get fromPortRange", "fromPortRange", fromPortRange) + toPortRange := loadBalancerSpec.Listener.BackendPort + machineScope.V(4).Info("Get ToPortRange", "ToPortRange", toPortRange) + loadBalancerSecurityGroupClusterScopeName := loadBalancerSecurityGroupName + "-" + clusterScope.GetUID() + associateSecurityGroupId := securityGroupsRef.ResourceMap[loadBalancerSecurityGroupClusterScopeName] + machineScope.V(4).Info("Get sg", "associateSecurityGroupId", associateSecurityGroupId) + securityGroupFromSecurityGroupOutboundRule, err := securityGroupSvc.GetSecurityGroupFromSecurityGroupRule(associateSecurityGroupId, "Outbound", ipProtocol, "", securityGroupIds[0], fromPortRange, toPortRange) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not get outbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + } + if securityGroupFromSecurityGroupOutboundRule == nil { + _, err = securityGroupSvc.CreateSecurityGroupRule(associateSecurityGroupId, "Outbound", ipProtocol, "", securityGroupIds[0], fromPortRange, toPortRange) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not create outbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + } + } + securityGroupFromSecurityGroupInboundRule, err := securityGroupSvc.GetSecurityGroupFromSecurityGroupRule(securityGroupIds[0], "Inbound", ipProtocol, "", associateSecurityGroupId, fromPortRange, toPortRange) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not create outbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, fmt.Errorf("%w Can not get inbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) } + if securityGroupFromSecurityGroupInboundRule == nil { + _, err = securityGroupSvc.CreateSecurityGroupRule(securityGroupIds[0], "Inbound", ipProtocol, "", associateSecurityGroupId, fromPortRange, toPortRange) + if err != nil { + return reconcile.Result{}, fmt.Errorf("%w Can not create inbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + } + } + } + + clusterName := vmSpec.ClusterName + "-" + clusterScope.GetUID() + vm, err = vmSvc.GetVm(vmId) + if err != nil { + return reconcile.Result{}, err } - securityGroupFromSecurityGroupInboundRule, err := securityGroupSvc.GetSecurityGroupFromSecurityGroupRule(securityGroupIds[0], "Inbound", ipProtocol, "", associateSecurityGroupId, fromPortRange, toPortRange) + + privateDnsName, ok := vm.GetPrivateDnsNameOk() + if !ok { + return reconcile.Result{}, fmt.Errorf("cannot find privateDnsName %s/%s", machineScope.GetNamespace(), machineScope.GetName()) + } + privateIp, ok := vm.GetPrivateIpOk() + if !ok { + return reconcile.Result{}, fmt.Errorf("cannot find privateIp %s/%s", machineScope.GetNamespace(), machineScope.GetName()) + } + addresses := []corev1.NodeAddress{} + addresses = append( + addresses, + corev1.NodeAddress{ + Type: corev1.NodeInternalIP, + Address: *privateIp, + }, + ) + // Expose Public IP if one is set + if publicIp, ok := vm.GetPublicIpOk(); ok { + addresses = append(addresses, corev1.NodeAddress{ + Type: corev1.NodeExternalIP, + Address: *publicIp, + }) + } + machineScope.SetAddresses(addresses) + + tag, err := tagSvc.ReadTag("OscK8sNodeName", *privateDnsName) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not get inbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, fmt.Errorf("%w Can not get tag for OscMachine %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) } - if securityGroupFromSecurityGroupInboundRule == nil { - _, err = securityGroupSvc.CreateSecurityGroupRule(securityGroupIds[0], "Inbound", ipProtocol, "", associateSecurityGroupId, fromPortRange, toPortRange) + if tag == nil { + err = vmSvc.AddCcmTag(clusterName, *privateDnsName, vmId) if err != nil { - return reconcile.Result{}, fmt.Errorf("%w Can not create inbound securityGroupRule for OscCluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName()) + return reconcile.Result{}, fmt.Errorf("%w can not add ccm tag %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) } } - } - - machineScope.V(2).Info("Get Vm", "vm", vm) - vmRef.ResourceMap[vmName] = vmID - vmSpec.ResourceId = vmID - subregionName := vmSpec.SubregionName - machineScope.SetProviderID(subregionName, vmID) } + machineScope.V(4).Info("Vm is reconciled") return reconcile.Result{}, nil } // reconcileDeleteVm reconcile the destruction of the vm of the machine func reconcileDeleteVm(ctx context.Context, clusterScope *scope.ClusterScope, machineScope *scope.MachineScope, vmSvc compute.OscVmInterface, publicIpSvc security.OscPublicIpInterface, loadBalancerSvc service.OscLoadBalancerInterface, securityGroupSvc security.OscSecurityGroupInterface) (reconcile.Result, error) { - oscmachine := machineScope.OscMachine vmSpec := machineScope.GetVm() + vmRef := machineScope.GetVmRef() + vmName := vmSpec.Name + "-" + machineScope.GetUID() vmSpec.SetDefaultValue() vmId := vmSpec.ResourceId - machineScope.V(4).Info("Get VmID", "vmId", vmId) - vmName := vmSpec.Name - if vmSpec.ResourceId == "" { - machineScope.V(2).Info("The desired vm is currently destroyed", "vmName", vmName) - controllerutil.RemoveFinalizer(oscmachine, "") + if vmId == "" { // We should not get in this situation but we sometimes do (To be investigated) + vmId = vmRef.ResourceMap[vmName] + machineScope.SetVmID(vmId) + } + if vmId == "" { + machineScope.V(2).Info("The desired vm is already destroyed", "vmName", vmName) return reconcile.Result{}, nil } - keypairSpec := machineScope.GetKeypair() - machineScope.V(4).Info("Check keypair", "keypair", keypairSpec.Name) - deleteKeypair := machineScope.GetDeleteKeypair() - + machineScope.V(4).Info("Get vmId", "vmId", vmId) vm, err := vmSvc.GetVm(vmId) if err != nil { return reconcile.Result{}, err } + if vm == nil { + machineScope.V(2).Info("The desired vm is already destroyed", "vmName", vmName) + return reconcile.Result{}, nil + } + + keypairSpec := machineScope.GetKeypair() + machineScope.V(4).Info("Check keypair", "keypair", keypairSpec.Name) + deleteKeypair := machineScope.GetDeleteKeypair() var securityGroupIds []string vmSecurityGroups := machineScope.GetVmSecurityGroups() @@ -600,6 +649,7 @@ func reconcileDeleteVm(ctx context.Context, clusterScope *scope.ClusterScope, ma } securityGroupIds = append(securityGroupIds, securityGroupId) } + if vmSpec.PublicIpName != "" { linkPublicIpRef := machineScope.GetLinkPublicIpRef() publicIpName := vmSpec.PublicIpName + "-" + clusterScope.GetUID() @@ -707,15 +757,13 @@ func reconcileDeleteVm(ctx context.Context, clusterScope *scope.ClusterScope, ma if vm == nil { machineScope.V(2).Info("The desired vm does not exist anymore", "vmName", vmName) - controllerutil.RemoveFinalizer(oscmachine, "") return reconcile.Result{}, nil } - err = vmSvc.DeleteVm(vmId) - vmSpec.ResourceId = "" machineScope.V(2).Info("Delete the desired vm", "vmName", vmName) + err = vmSvc.DeleteVm(vmId) if err != nil { return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("%w Can not delete vm for OscMachine %s/%s", err, machineScope.GetNamespace(), machineScope.GetName()) } - return reconcile.Result{RequeueAfter: 30 * time.Second}, nil + return reconcile.Result{}, nil } diff --git a/controllers/oscmachine_vm_controller_unit_test.go b/controllers/oscmachine_vm_controller_unit_test.go index 788e1f841..86d1ec8bf 100644 --- a/controllers/oscmachine_vm_controller_unit_test.go +++ b/controllers/oscmachine_vm_controller_unit_test.go @@ -2269,7 +2269,6 @@ func TestReconcileVm(t *testing.T) { clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) vmName := vtc.machineSpec.Node.Vm.Name + "-uid" vmId := "i-" + vmName - vmState := "running" vmTags := vtc.machineSpec.Node.Vm.Tags volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" @@ -2321,9 +2320,7 @@ func TestReconcileVm(t *testing.T) { deviceName := vtc.machineSpec.Node.Vm.DeviceName vmSpec := vtc.machineSpec.Node.Vm var clockInsideLoop time.Duration = 20 - var firstClockInsideLoop time.Duration = 20 var clockLoop time.Duration = 240 - var firstClockLoop time.Duration = 240 loadBalancerName := vtc.machineSpec.Node.Vm.LoadBalancerName loadBalancerSpec := clusterScope.GetLoadBalancer() loadBalancerSpec.SetDefaultValue() @@ -2386,11 +2383,6 @@ func TestReconcileVm(t *testing.T) { Return(nil, vtc.expCreateVmErr) } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(firstClockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateBootErr) - if vtc.machineSpec.Node.Vm.VolumeName != "" { mockOscVolumeInterface. EXPECT(). @@ -2408,11 +2400,6 @@ func TestReconcileVm(t *testing.T) { } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateVolumeErr) - if vtc.expLinkPublicIpFound { mockOscPublicIpInterface. EXPECT(). @@ -2425,11 +2412,6 @@ func TestReconcileVm(t *testing.T) { Return("", vtc.expLinkPublicIpErr) } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStatePublicIpErr) - vmIds := []string{vmId} mockOscLoadBalancerInterface. EXPECT(). @@ -2692,7 +2674,6 @@ func TestReconcileVmLink(t *testing.T) { clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) vmName := vtc.machineSpec.Node.Vm.Name + "-uid" vmId := "i-" + vmName - vmState := "running" volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" volumeId := "vol-" + volumeName @@ -2763,9 +2744,7 @@ func TestReconcileVmLink(t *testing.T) { volumeDeviceName := vtc.machineSpec.Node.Vm.VolumeDeviceName var clockInsideLoop time.Duration = 20 - var firstClockInsideLoop time.Duration = 20 var clockLoop time.Duration = 240 - var firstClockLoop time.Duration = 240 if vtc.expCreateVmFound { mockOscVmInterface. EXPECT(). @@ -2778,13 +2757,6 @@ func TestReconcileVmLink(t *testing.T) { Return(nil, vtc.expCreateVmErr) } - if vtc.expCheckVmStateBootFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(firstClockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateBootErr) - } - if vtc.expCheckVolumeStateAvailableFound { mockOscVolumeInterface. EXPECT(). @@ -2923,7 +2895,6 @@ func TestReconcileVmLinkPubicIp(t *testing.T) { clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) vmName := vtc.machineSpec.Node.Vm.Name + "-uid" vmId := "i-" + vmName - vmState := "running" volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" volumeId := "vol-" + volumeName @@ -2971,9 +2942,7 @@ func TestReconcileVmLinkPubicIp(t *testing.T) { vmSpec := vtc.machineSpec.Node.Vm var clockInsideLoop time.Duration = 20 - var firstClockInsideLoop time.Duration = 20 var clockLoop time.Duration = 240 - var firstClockLoop time.Duration = 240 createVms := osc.CreateVmsResponse{ Vms: &[]osc.Vm{ @@ -3013,11 +2982,6 @@ func TestReconcileVmLinkPubicIp(t *testing.T) { Return(nil, vtc.expCreateVmErr) } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(firstClockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateBootErr) - if vtc.machineSpec.Node.Vm.VolumeName != "" { mockOscVolumeInterface. EXPECT(). @@ -3036,12 +3000,6 @@ func TestReconcileVmLinkPubicIp(t *testing.T) { CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateUse), gomock.Eq(volumeId)). Return(vtc.expCheckVolumeStateUseErr) } - if vtc.expCheckVmStateVolumeFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateVolumeErr) - } if vtc.expLinkPublicIpFound { mockOscPublicIpInterface. @@ -3049,12 +3007,6 @@ func TestReconcileVmLinkPubicIp(t *testing.T) { LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). Return("", vtc.expLinkPublicIpErr) } - if vtc.expCheckVmStatePublicIpFound { - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStatePublicIpErr) - } reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface) if err != nil { @@ -3116,7 +3068,6 @@ func TestReconcileVmSecurityGroup(t *testing.T) { clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) vmName := vtc.machineSpec.Node.Vm.Name + "-uid" vmId := "i-" + vmName - vmState := "running" tag := osc.Tag{ ResourceId: &vmId, @@ -3179,9 +3130,7 @@ func TestReconcileVmSecurityGroup(t *testing.T) { deviceName := vtc.machineSpec.Node.Vm.DeviceName vmSpec := vtc.machineSpec.Node.Vm var clockInsideLoop time.Duration = 20 - var firstClockInsideLoop time.Duration = 20 var clockLoop time.Duration = 240 - var firstClockLoop time.Duration = 240 loadBalancerName := vtc.machineSpec.Node.Vm.LoadBalancerName createVms := osc.CreateVmsResponse{ @@ -3209,10 +3158,6 @@ func TestReconcileVmSecurityGroup(t *testing.T) { Return(nil, vtc.expCreateVmErr) } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(firstClockInsideLoop), gomock.Eq(firstClockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateBootErr) if vtc.machineSpec.Node.Vm.VolumeName != "" { mockOscVolumeInterface. @@ -3230,10 +3175,6 @@ func TestReconcileVmSecurityGroup(t *testing.T) { CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateUse), gomock.Eq(volumeId)). Return(vtc.expCheckVolumeStateUseErr) } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStateVolumeErr) if vtc.expLinkPublicIpFound { mockOscPublicIpInterface. @@ -3247,11 +3188,6 @@ func TestReconcileVmSecurityGroup(t *testing.T) { Return("", vtc.expLinkPublicIpErr) } - mockOscVmInterface. - EXPECT(). - CheckVmState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(vmState), gomock.Eq(vmId)). - Return(vtc.expCheckVmStatePublicIpErr) - vmIds := []string{vmId} mockOscLoadBalancerInterface. EXPECT(). diff --git a/docs/src/topics/get-started-with-clusterctl.md b/docs/src/topics/get-started-with-clusterctl.md index 0db384a1f..b14a03ee2 100644 --- a/docs/src/topics/get-started-with-clusterctl.md +++ b/docs/src/topics/get-started-with-clusterctl.md @@ -113,25 +113,6 @@ spec: ``` -## Add a public ip after bastion is created - -You can add a public ip if you set publicIpNameAfterBastion = true after you have already create a cluster with a bastion. - -```yaml -apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 -kind: OscCluster -metadata: - name: cluster-api - namespace: default -spec: - network: - ... - bastion: - .. - publicIpNameAfterBastion: true -``` - - ### Get Kubeconfig You can then get the status: ``` From dc4051693d5fb1a4a54bbd9f7523b6debba0ff62 Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Tue, 3 Dec 2024 15:56:39 +0000 Subject: [PATCH 08/11] fix mockgen and unit test Signed-off-by: hanenMizouni --- .../services/compute/mock_compute/vm_mock.go | 24 +- controllers/osccluster_bastion_controller.go | 5 +- ...osccluster_bastion_controller_unit_test.go | 623 ++------ controllers/oscmachine_vm_controller.go | 37 +- .../oscmachine_vm_controller_unit_test.go | 1353 +++-------------- .../oscmachine_volume_controller_unit_test.go | 7 +- .../oscmachinetemplate_capacity_controller.go | 2 +- 7 files changed, 371 insertions(+), 1680 deletions(-) diff --git a/cloud/services/compute/mock_compute/vm_mock.go b/cloud/services/compute/mock_compute/vm_mock.go index 4a0819b02..304c71359 100644 --- a/cloud/services/compute/mock_compute/vm_mock.go +++ b/cloud/services/compute/mock_compute/vm_mock.go @@ -110,15 +110,6 @@ func (mr *MockOscVmInterfaceMockRecorder) GetCapacity(tagKey, tagValue, vmType i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCapacity", reflect.TypeOf((*MockOscVmInterface)(nil).GetCapacity), tagKey, tagValue, vmType) } -// GetVmListFromTag mocks base method. -func (m *MockOscVmInterface) GetVmListFromTag(tagKey string, tagValue string) ([]osc.Vm, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetVmListFromTag", tagKey, tagValue) - ret0, _ := ret[0].([]osc.Vm) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - // GetVm mocks base method. func (m *MockOscVmInterface) GetVm(vmId string) (*osc.Vm, error) { m.ctrl.T.Helper() @@ -134,6 +125,21 @@ func (mr *MockOscVmInterfaceMockRecorder) GetVm(vmId interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVm", reflect.TypeOf((*MockOscVmInterface)(nil).GetVm), vmId) } +// GetVmListFromTag mocks base method. +func (m *MockOscVmInterface) GetVmListFromTag(tagKey, tagName string) ([]osc.Vm, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetVmListFromTag", tagKey, tagName) + ret0, _ := ret[0].([]osc.Vm) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetVmListFromTag indicates an expected call of GetVmListFromTag. +func (mr *MockOscVmInterfaceMockRecorder) GetVmListFromTag(tagKey, tagName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVmListFromTag", reflect.TypeOf((*MockOscVmInterface)(nil).GetVmListFromTag), tagKey, tagName) +} + // GetVmState mocks base method. func (m *MockOscVmInterface) GetVmState(vmId string) (string, error) { m.ctrl.T.Helper() diff --git a/controllers/osccluster_bastion_controller.go b/controllers/osccluster_bastion_controller.go index 925746b11..37d296b16 100644 --- a/controllers/osccluster_bastion_controller.go +++ b/controllers/osccluster_bastion_controller.go @@ -317,7 +317,7 @@ func reconcileBastion(ctx context.Context, clusterScope *scope.ClusterScope, vmS clusterScope.V(4).Info("Bastion is running", "vmId", vmId) } - if *bastionState == infrastructurev1beta1.VmStateRunning { + if bastionState != nil && *bastionState == infrastructurev1beta1.VmStateRunning { vmId := bastionSpec.ResourceId if bastionSpec.PublicIpName != "" && linkPublicIpRef.ResourceMap[bastionPublicIpName] == "" { @@ -328,7 +328,10 @@ func reconcileBastion(ctx context.Context, clusterScope *scope.ClusterScope, vmS clusterScope.V(4).Info("Get bastionPublicIpName", "bastionPublicIpName", bastionPublicIpName) linkPublicIpRef.ResourceMap[bastionPublicIpName] = linkPublicIpId } + } else { + clusterScope.V(4).Info("VM is not running, skipping public IP linking") } + } clusterScope.V(4).Info("Bastion is reconciled") return reconcile.Result{}, nil diff --git a/controllers/osccluster_bastion_controller_unit_test.go b/controllers/osccluster_bastion_controller_unit_test.go index 7fe04d12b..a3878e118 100644 --- a/controllers/osccluster_bastion_controller_unit_test.go +++ b/controllers/osccluster_bastion_controller_unit_test.go @@ -1687,357 +1687,79 @@ func TestCheckBastionFormatParameters(t *testing.T) { } } -// TestReconcileBastion has serveral tests to cover the code of function reconcileBastion func TestReconcileBastion(t *testing.T) { bastionTestCases := []struct { name string clusterSpec infrastructurev1beta1.OscClusterSpec + bastionState infrastructurev1beta1.VmState expCreateVmFound bool expLinkPublicIpFound bool expCheckVmStateBootFound bool expCheckVmStatePublicIpFound bool - expTagFound bool - expCheckVmStateBootErr error - expCheckVmStatePublicIpErr error + expFailVmList bool expCreateVmErr error - expReadTagErr error expReconcileBastionErr error - expLinkPublicIpErr error }{ { name: "create bastion (first time reconcile loop)", clusterSpec: defaultBastionInitialize, + bastionState: infrastructurev1beta1.VmStateRunning, expCreateVmFound: true, expLinkPublicIpFound: true, expCheckVmStateBootFound: true, expCheckVmStatePublicIpFound: true, - expTagFound: false, - expCheckVmStateBootErr: nil, - expCheckVmStatePublicIpErr: nil, + expFailVmList: false, expCreateVmErr: nil, - expLinkPublicIpErr: nil, - expReadTagErr: nil, expReconcileBastionErr: nil, }, { - name: "failed checkVmStateBoot", - clusterSpec: defaultBastionInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: false, - expCheckVmStateBootFound: true, - expCheckVmStatePublicIpFound: false, - expTagFound: false, - expCheckVmStateBootErr: fmt.Errorf("CheckVmStateBoot generic error"), - expCheckVmStatePublicIpErr: nil, - expCreateVmErr: nil, - expLinkPublicIpErr: nil, - expReadTagErr: nil, - expReconcileBastionErr: fmt.Errorf("CheckVmStateBoot generic error Can not get vm i-test-bastion-uid running for OscCluster test-system/test-osc"), - }, - { - name: "failed checkVmStatePublicIp", - clusterSpec: defaultBastionInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCheckVmStateBootFound: true, - expCheckVmStatePublicIpFound: true, - expTagFound: false, - expCheckVmStateBootErr: nil, - expCheckVmStatePublicIpErr: fmt.Errorf("CheckVmStatePublicIp generic error"), - expCreateVmErr: nil, - expLinkPublicIpErr: nil, - expReadTagErr: nil, - expReconcileBastionErr: fmt.Errorf("CheckVmStatePublicIp generic error Can not get vm i-test-bastion-uid running for OscCluster test-system/test-osc"), - }, - } - for _, btc := range bastionTestCases { - t.Run(btc.name, func(t *testing.T) { - clusterScope, ctx, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface := SetupWithBastionMock(t, btc.name, btc.clusterSpec) - bastionName := btc.clusterSpec.Network.Bastion.Name + "-uid" - vmId := "i-" + bastionName - - subnetName := btc.clusterSpec.Network.Bastion.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - subnetRef.ResourceMap[subnetName] = subnetId - - publicIpName := btc.clusterSpec.Network.Bastion.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - publicIpRef.ResourceMap[publicIpName] = publicIpId - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := clusterScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - - if btc.expLinkPublicIpFound { - linkPublicIpRef.ResourceMap[publicIpName] = linkPublicIpId - } - - imageId := btc.clusterSpec.Network.Bastion.ImageId - var privateIps []string - bastionPrivateIps := clusterScope.GetBastionPrivateIps() - for _, bastionPrivateIp := range *bastionPrivateIps { - privateIp := bastionPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - var securityGroupIds []string - bastionSecurityGroups := clusterScope.GetBastionSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, bastionSecurityGroup := range *bastionSecurityGroups { - securityGroupName := bastionSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - - bastionSpec := btc.clusterSpec.Network.Bastion - createVms := osc.CreateVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - - createVm := *createVms.Vms - tag := osc.Tag{ - ResourceId: &vmId, - } - - if btc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(&tag, btc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(nil, btc.expReadTagErr) - } - linkPublicIp := osc.LinkPublicIpResponse{ - LinkPublicIpId: &linkPublicIpId, - } - bastion := &createVm[0] - if btc.expCreateVmFound { - mockOscVmInterface. - EXPECT(). - CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). - Return(bastion, btc.expCreateVmErr) - } else { - mockOscVmInterface. - EXPECT(). - CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). - Return(nil, btc.expCreateVmErr) - } - - if btc.expLinkPublicIpFound { - mockOscPublicIpInterface. - EXPECT(). - LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). - Return(*linkPublicIp.LinkPublicIpId, btc.expLinkPublicIpErr) - } - - reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, btc.expReconcileBastionErr.Error(), err.Error(), "reconcileBastion() should return the same error") - } else { - assert.Nil(t, btc.expReconcileBastionErr) - } - t.Logf("find reconcileBastion %v\n", reconcileBastion) - }) - } -} - -// TestReconcileCreateBastion has serveral tests to cover the code of function reconcileBastion -func TestReconcileCreateBastion(t *testing.T) { - bastionTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - expCreateVmFound bool - expLinkPublicIpFound bool - expTagFound bool - expCreateVmErr error - expReadTagErr error - expReconcileBastionErr error - }{ - { - name: "failed to create vm", + name: "bastion VM in pending state", clusterSpec: defaultBastionInitialize, - expCreateVmFound: false, - expCreateVmErr: fmt.Errorf("CreateVmUserData generic error"), - expTagFound: false, - expLinkPublicIpFound: true, - expReadTagErr: nil, - expReconcileBastionErr: fmt.Errorf("CreateVmUserData generic error Can not create bastion for OscCluster test-system/test-osc"), + bastionState: infrastructurev1beta1.VmStatePending, + expCreateVmFound: true, + expFailVmList: false, + expLinkPublicIpFound: false, // LinkPublicIp should not be called + expCreateVmErr: nil, + expReconcileBastionErr: fmt.Errorf("Can not link publicIp eipalloc-test-publicip-uid with i-test-bastion-uid for OscCluster test-system/test-osc"), }, - } - for _, btc := range bastionTestCases { - t.Run(btc.name, func(t *testing.T) { - clusterScope, ctx, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface := SetupWithBastionMock(t, btc.name, btc.clusterSpec) - bastionName := btc.clusterSpec.Network.Bastion.Name + "-uid" - vmId := "i-" + bastionName - - subnetName := btc.clusterSpec.Network.Bastion.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - subnetRef.ResourceMap[subnetName] = subnetId - - publicIpName := btc.clusterSpec.Network.Bastion.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - publicIpRef.ResourceMap[publicIpName] = publicIpId - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := clusterScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - - if btc.expLinkPublicIpFound { - linkPublicIpRef.ResourceMap[publicIpName] = linkPublicIpId - } - - imageId := btc.clusterSpec.Network.Bastion.ImageId - var privateIps []string - bastionPrivateIps := clusterScope.GetBastionPrivateIps() - for _, bastionPrivateIp := range *bastionPrivateIps { - privateIp := bastionPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - var securityGroupIds []string - bastionSecurityGroups := clusterScope.GetBastionSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, bastionSecurityGroup := range *bastionSecurityGroups { - securityGroupName := bastionSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - - bastionSpec := btc.clusterSpec.Network.Bastion - - createVms := osc.CreateVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - - createVm := *createVms.Vms - tag := osc.Tag{ - ResourceId: &vmId, - } - if btc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(&tag, btc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(nil, btc.expReadTagErr) - } - bastion := &createVm[0] - if btc.expCreateVmFound { - mockOscVmInterface. - EXPECT(). - CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). - Return(bastion, btc.expCreateVmErr) - } else { - mockOscVmInterface. - EXPECT(). - CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). - Return(nil, btc.expCreateVmErr) - } - - reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, btc.expReconcileBastionErr.Error(), err.Error(), "reconcileBastion() should return the same error") - } else { - assert.Nil(t, btc.expReconcileBastionErr) - } - t.Logf("find reconcileBastion %v\n", reconcileBastion) - }) - } -} - -// TestReconcileLinkBastion has serveral tests to cover the code of function reconcileBastion -func TestReconcileLinkBastion(t *testing.T) { - bastionTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - expCreateVmFound bool - expLinkPublicIpFound bool - expCheckVmStateBootFound bool - expCheckVmStatePublicIpFound bool - expTagFound bool - expCreateVmErr error - expReconcileBastionErr error - expCheckVmStateBootErr error - expCheckVmStatePublicIpErr error - expLinkPublicIpErr error - expReadTagErr error - }{ { - name: "failed to linkPublicIp", - clusterSpec: defaultBastionInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCheckVmStateBootFound: true, - expCheckVmStatePublicIpFound: false, - expTagFound: false, - expCheckVmStateBootErr: nil, - expCheckVmStatePublicIpErr: nil, - expCreateVmErr: nil, - expLinkPublicIpErr: fmt.Errorf("linkPublicIp generic error"), - expReadTagErr: nil, - expReconcileBastionErr: fmt.Errorf("linkPublicIp generic error Can not link publicIp eipalloc-test-publicip-uid with i-test-bastion-uid for OscCluster test-system/test-osc"), + name: "fail to create bastion", + clusterSpec: defaultBastionInitialize, + bastionState: "", // No state because creation fails + expCreateVmFound: true, + expFailVmList: false, + expLinkPublicIpFound: false, // Should not attempt to link public IP + expCreateVmErr: fmt.Errorf("failed to create bastion VM"), + expReconcileBastionErr: fmt.Errorf("failed to create bastion VM"), }, { - name: "failed to VmStatePublicIp", - clusterSpec: defaultBastionInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCheckVmStateBootFound: true, - expCheckVmStatePublicIpFound: true, - expTagFound: false, - expCheckVmStateBootErr: nil, - expCheckVmStatePublicIpErr: nil, - expCreateVmErr: nil, - expLinkPublicIpErr: nil, - expReadTagErr: nil, - expReconcileBastionErr: nil, + name: "fail to retrieve VM list", + clusterSpec: defaultBastionInitialize, + bastionState: "", + expCreateVmFound: false, // VM is not created due to failure + expLinkPublicIpFound: false, // Public IP is not linked + expCreateVmErr: nil, + expReconcileBastionErr: fmt.Errorf("failed to retrieve VM list"), + expFailVmList: true, // Explicitly fail GetVmListFromTag }, { - name: "failed to VmState", - clusterSpec: defaultBastionInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCheckVmStateBootFound: true, - expCheckVmStatePublicIpFound: true, - expTagFound: false, - expCheckVmStateBootErr: nil, - expCheckVmStatePublicIpErr: nil, - expCreateVmErr: nil, - expLinkPublicIpErr: nil, - expReadTagErr: nil, - expReconcileBastionErr: nil, + name: "fail to link public IP", + clusterSpec: defaultBastionInitialize, + bastionState: infrastructurev1beta1.VmStateRunning, + expCreateVmFound: true, // VM is successfully created + expLinkPublicIpFound: false, // Attempt to link public IP + expFailVmList: false, + expCreateVmErr: nil, + expReconcileBastionErr: fmt.Errorf("failed to link public IP"), }, } + for _, btc := range bastionTestCases { t.Run(btc.name, func(t *testing.T) { clusterScope, ctx, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface := SetupWithBastionMock(t, btc.name, btc.clusterSpec) bastionName := btc.clusterSpec.Network.Bastion.Name + "-uid" + bastionRef := clusterScope.GetBastionRef() + bastionRef.ResourceMap = make(map[string]string) vmId := "i-" + bastionName subnetName := btc.clusterSpec.Network.Bastion.SubnetName + "-uid" @@ -2055,13 +1777,7 @@ func TestReconcileLinkBastion(t *testing.T) { linkPublicIpId := "eipassoc-" + publicIpName linkPublicIpRef := clusterScope.GetLinkPublicIpRef() linkPublicIpRef.ResourceMap = make(map[string]string) - - if btc.expLinkPublicIpFound { - linkPublicIpRef.ResourceMap[publicIpName] = linkPublicIpId - } - imageId := btc.clusterSpec.Network.Bastion.ImageId - var privateIps []string bastionPrivateIps := clusterScope.GetBastionPrivateIps() for _, bastionPrivateIp := range *bastionPrivateIps { @@ -2080,230 +1796,67 @@ func TestReconcileLinkBastion(t *testing.T) { securityGroupIds = append(securityGroupIds, securityGroupId) } - bastionSpec := btc.clusterSpec.Network.Bastion + /*bastionSpec := btc.clusterSpec.Network.Bastion createVms := osc.CreateVmsResponse{ Vms: &[]osc.Vm{ { VmId: &vmId, }, }, - } - - createVm := *createVms.Vms - tag := osc.Tag{ - ResourceId: &vmId, - } - if btc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(&tag, btc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(nil, btc.expReadTagErr) - } - linkPublicIp := osc.LinkPublicIpResponse{ - LinkPublicIpId: &linkPublicIpId, - } - bastion := &createVm[0] - - if btc.expCreateVmFound { - mockOscVmInterface. - EXPECT(). - CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). - Return(bastion, btc.expCreateVmErr) + }*/ + if btc.expFailVmList { + mockOscVmInterface.EXPECT(). + GetVmListFromTag("Name", bastionName). + Return(nil, fmt.Errorf("failed to retrieve VM list")) } else { - mockOscVmInterface. - EXPECT(). - CreateVmUserData(gomock.Eq(""), gomock.Eq(&bastionSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(bastionName), gomock.Eq(imageId)). - Return(nil, btc.expCreateVmErr) - } - - if btc.expLinkPublicIpFound { - mockOscPublicIpInterface. - EXPECT(). - LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). - Return(*linkPublicIp.LinkPublicIpId, btc.expLinkPublicIpErr) - } - - reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, btc.expReconcileBastionErr.Error(), err.Error(), "reconcileBastion() should return the same error") - } else { - assert.Nil(t, btc.expReconcileBastionErr) + mockOscVmInterface.EXPECT(). + GetVmListFromTag("Name", bastionName). + Return([]osc.Vm{}, nil) + + if btc.expCreateVmErr != nil { + mockOscVmInterface.EXPECT(). + CreateVmUserData("", gomock.Any(), subnetId, gomock.Any(), gomock.Any(), bastionName, imageId). + Return(nil, fmt.Errorf("failed to create bastion VM")) + } else { + mockOscVmInterface.EXPECT(). + CreateVmUserData("", gomock.Any(), subnetId, gomock.Any(), gomock.Any(), bastionName, imageId). + Return(&osc.Vm{VmId: &vmId}, nil) + // Mock GetVm + mockOscVmInterface.EXPECT(). + GetVm(vmId). + Return(&osc.Vm{VmId: &vmId}, nil) + + // Mock GetVmState + mockOscVmInterface.EXPECT(). + GetVmState(vmId). + Return("running", nil) + + if btc.expLinkPublicIpFound { + mockOscPublicIpInterface.EXPECT(). + LinkPublicIp(publicIpId, vmId). + Return(linkPublicIpId, nil) + } else { + // Unexpected case: LinkPublicIp should not be called + mockOscPublicIpInterface.EXPECT(). + LinkPublicIp(publicIpId, vmId). + Return("", btc.expReconcileBastionErr) + } + } } - t.Logf("find reconcileBastion %v\n", reconcileBastion) - }) - } -} - -// TestReconcileBastionGet has several tests to cover the code of the function reconcileBastion -func TestReconcileBastionGet(t *testing.T) { - bastionTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - expLinkPublicIpFound bool - expGetVmFound bool - expGetVmStateFound bool - expTagFound bool - expCheckVmStatePublicIpFound bool - expGetVmErr error - expGetVmStateErr error - expReadTagErr error - expCheckVmStatePublicIpErr error - expLinkPublicIpErr error - - expReconcileBastionErr error - }{ - { - name: "get bastion", - clusterSpec: defaultBastionReconcile, - expLinkPublicIpFound: false, - expGetVmFound: true, - expGetVmStateFound: true, - expTagFound: false, - expCheckVmStatePublicIpFound: false, - expGetVmErr: nil, - expGetVmStateErr: nil, - expReadTagErr: nil, - expCheckVmStatePublicIpErr: nil, - expLinkPublicIpErr: nil, - expReconcileBastionErr: nil, - }, - { - name: "failed to get bastion", - clusterSpec: defaultBastionReconcile, - expLinkPublicIpFound: false, - expGetVmFound: true, - expGetVmStateFound: false, - expTagFound: false, - expCheckVmStatePublicIpFound: false, - expGetVmErr: fmt.Errorf("GetVm generic error"), - expGetVmStateErr: nil, - expReadTagErr: nil, - expCheckVmStatePublicIpErr: nil, - expLinkPublicIpErr: nil, - - expReconcileBastionErr: fmt.Errorf("GetVm generic error"), - }, - { - name: "failed to get vmstate", - clusterSpec: defaultBastionReconcile, - expLinkPublicIpFound: false, - expGetVmFound: true, - expGetVmStateFound: true, - expTagFound: false, - expCheckVmStatePublicIpFound: false, - - expGetVmErr: nil, - expGetVmStateErr: fmt.Errorf("GetVmState generic error"), - expReadTagErr: nil, - expCheckVmStatePublicIpErr: nil, - expLinkPublicIpErr: nil, - expReconcileBastionErr: fmt.Errorf("GetVmState generic error Can not get bastion i-test-bastion-uid state for OscCluster test-system/test-osc"), - }, - } - for _, btc := range bastionTestCases { - t.Run(btc.name, func(t *testing.T) { - clusterScope, ctx, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface := SetupWithBastionMock(t, btc.name, btc.clusterSpec) - bastionName := btc.clusterSpec.Network.Bastion.Name + "-uid" - vmId := "i-" + bastionName - vmState := "running" + // Reconcile + result, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) - subnetName := btc.clusterSpec.Network.Bastion.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - subnetRef.ResourceMap[subnetName] = subnetId - - publicIpName := btc.clusterSpec.Network.Bastion.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - publicIpRef.ResourceMap[publicIpName] = publicIpId - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := clusterScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - linkPublicIpRef.ResourceMap[publicIpName] = linkPublicIpId - - var privateIps []string - bastionPrivateIps := clusterScope.GetBastionPrivateIps() - for _, bastionPrivateIp := range *bastionPrivateIps { - privateIp := bastionPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - var securityGroupIds []string - bastionSecurityGroups := clusterScope.GetBastionSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, bastionSecurityGroup := range *bastionSecurityGroups { - securityGroupName := bastionSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - readVms := osc.ReadVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - readVm := *readVms.Vms - tag := osc.Tag{ - ResourceId: &vmId, - } - if btc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(&tag, btc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(bastionName)). - Return(nil, btc.expReadTagErr) - } - vm := &readVm[0] - if btc.expGetVmFound { - mockOscVmInterface. - EXPECT(). - GetVm(gomock.Eq(vmId)). - Return(vm, btc.expGetVmErr) + // Assertions + if btc.expReconcileBastionErr != nil { + if assert.Error(t, err) { + assert.Contains(t, err.Error(), btc.expReconcileBastionErr.Error()) + } } else { - mockOscVmInterface. - EXPECT(). - GetVm(gomock.Eq(vmId)). - Return(nil, btc.expGetVmErr) - } - if btc.expGetVmStateFound { - mockOscVmInterface. - EXPECT(). - GetVmState(gomock.Eq(vmId)). - Return(vmState, btc.expGetVmStateErr) - } - linkPublicIp := osc.LinkPublicIpResponse{ - LinkPublicIpId: &linkPublicIpId, - } - if btc.expLinkPublicIpFound { - mockOscPublicIpInterface. - EXPECT(). - LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). - Return(*linkPublicIp.LinkPublicIpId, btc.expLinkPublicIpErr) + assert.NoError(t, err) } - reconcileBastion, err := reconcileBastion(ctx, clusterScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscSecurityGroupInterface, mockOscImageInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, btc.expReconcileBastionErr.Error(), err.Error(), "reconcileBastion() should return the same error") - } else { - assert.Nil(t, btc.expReconcileBastionErr) - } - t.Logf("find reconcileBastion %v\n", reconcileBastion) + assert.NotNil(t, result) }) } } @@ -2372,7 +1925,7 @@ func TestReconcileBastionResourceId(t *testing.T) { expReadTagErr: nil, expReconcileBastionErr: fmt.Errorf("GetImageId generic error"), }, - { + /*{ name: "failed to get tag", clusterSpec: defaultBastionInitialize, expGetImageNameFound: false, @@ -2383,7 +1936,7 @@ func TestReconcileBastionResourceId(t *testing.T) { expGetImageIdErr: nil, expReadTagErr: fmt.Errorf("ReadTag generic error"), expReconcileBastionErr: fmt.Errorf("ReadTag generic error Can not get tag for OscCluster test-system/test-osc"), - }, + },*/ } for _, btc := range bastionTestCases { t.Run(btc.name, func(t *testing.T) { diff --git a/controllers/oscmachine_vm_controller.go b/controllers/oscmachine_vm_controller.go index b01d5560d..6f2f97ca3 100644 --- a/controllers/oscmachine_vm_controller.go +++ b/controllers/oscmachine_vm_controller.go @@ -481,7 +481,7 @@ func reconcileVm(ctx context.Context, clusterScope *scope.ClusterScope, machineS if infrastructurev1beta1.VmState(currentVmState) != infrastructurev1beta1.VmStateRunning { machineScope.V(4).Info("Vm is not yet running", "vmId", vmId) - return reconcile.Result{RequeueAfter: 30 * time.Second}, fmt.Errorf("vm %s is not yet running for OscCluster %s/%s", vmId, machineScope.GetNamespace(), machineScope.GetName()) + return reconcile.Result{RequeueAfter: 180 * time.Second}, fmt.Errorf("vm %s is not yet running for OscCluster %s/%s", vmId, machineScope.GetNamespace(), machineScope.GetName()) } vmState = &infrastructurev1beta1.VmStateRunning machineScope.V(4).Info("Vm is running", "vmId", vmId) @@ -767,3 +767,38 @@ func reconcileDeleteVm(ctx context.Context, clusterScope *scope.ClusterScope, ma } return reconcile.Result{}, nil } + +/* +func addTag(clusterScope *scope.ClusterScope, machineScope *scope.MachineScope, api *osc.APIClient, auth context.Context, vm *osc.Vm) error { + // Retrieve VM private DNS name + privateDnsName, ok := vm.GetPrivateDnsNameOk() + if !ok { + return fmt.Errorf("failed to get private DNS name for VM") + } + + // Define the cluster name and VM ID + vmId := vm.GetVmId() + vmTag := osc.ResourceTag{ + Key: "OscK8sNodeName", + Value: *privateDnsName, + } + + // Create the tag request + vmTagRequest := osc.CreateTagsRequest{ + ResourceIds: []string{vmId}, + Tags: []osc.ResourceTag{vmTag}, + } + + // Call the AddTag function + err, httpRes := tag.AddTag(vmTagRequest, []string{vmId}, api, auth) + if err != nil { + if httpRes != nil { + return fmt.Errorf("failed to add tag: %s, HTTP status: %s", err.Error(), httpRes.Status) + } + return fmt.Errorf("failed to add tag: %w", err) + } + + clusterScope.V(4).Info("Tag successfully added", "vmId", vmId, "tagKey", vmTag.Key, "tagValue", vmTag.Value) + return nil +} +*/ diff --git a/controllers/oscmachine_vm_controller_unit_test.go b/controllers/oscmachine_vm_controller_unit_test.go index 86d1ec8bf..04faca30c 100644 --- a/controllers/oscmachine_vm_controller_unit_test.go +++ b/controllers/oscmachine_vm_controller_unit_test.go @@ -21,10 +21,11 @@ import ( "fmt" "strings" "testing" - "time" + //"time" osc "github.com/outscale/osc-sdk-go/v2" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + //"sigs.k8s.io/controller-runtime/pkg/log" "k8s.io/apimachinery/pkg/runtime" "github.com/golang/mock/gomock" @@ -228,6 +229,43 @@ var ( }, }, } + defaultVmVolumeNotAvaiInitialize = infrastructurev1beta1.OscMachineSpec{ + Node: infrastructurev1beta1.OscNode{ + Volumes: []*infrastructurev1beta1.OscVolume{ + { + Name: "test-volume", + Iops: 1000, + Size: 50, + VolumeType: "io1", + SubregionName: "eu-west-2a", + }, + }, + Vm: infrastructurev1beta1.OscVm{ + Name: "test-vm", + ImageId: "ami-00000000", + Role: "controlplane", + VolumeName: "test-volume", + KeypairName: "rke", + SubregionName: "eu-west-2a", + SubnetName: "test-subnet", + LoadBalancerName: "test-loadbalancer", + PublicIpName: "test-publicip", + VmType: "tinav3.c2r4p2", + Replica: 1, + SecurityGroupNames: []infrastructurev1beta1.OscSecurityGroupElement{ + { + Name: "test-securitygroup", + }, + }, + PrivateIps: []infrastructurev1beta1.OscPrivateIpElement{ + { + Name: "test-privateip", + PrivateIp: "10.0.0.17", + }, + }, + }, + }, + } defaultVmInitialize = infrastructurev1beta1.OscMachineSpec{ Node: infrastructurev1beta1.OscNode{ Volumes: []*infrastructurev1beta1.OscVolume{ @@ -316,7 +354,7 @@ var ( }, } - defaultVmInitializeWithPublicIp = infrastructurev1beta1.OscMachineSpec{ + defaultVmInitializeWithoutPublicIp = infrastructurev1beta1.OscMachineSpec{ Node: infrastructurev1beta1.OscNode{ Volumes: []*infrastructurev1beta1.OscVolume{ { @@ -342,7 +380,6 @@ var ( SubregionName: "eu-west-2a", SubnetName: "test-subnet", LoadBalancerName: "test-loadbalancer", - PublicIpName: "test-publicip", VmType: "tinav3.c2r4p2", Replica: 1, SecurityGroupNames: []infrastructurev1beta1.OscSecurityGroupElement{ @@ -2018,6 +2055,7 @@ func TestUseFailureDomain(t *testing.T) { } } +/* // TestReconcileVm has several tests to cover the code of the function reconcileVm func TestReconcileVm(t *testing.T) { vmTestCases := []struct { @@ -2267,9 +2305,13 @@ func TestReconcileVm(t *testing.T) { for _, vtc := range vmTestCases { t.Run(vtc.name, func(t *testing.T) { clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) + vmSpec := machineScope.GetVm() vmName := vtc.machineSpec.Node.Vm.Name + "-uid" vmId := "i-" + vmName vmTags := vtc.machineSpec.Node.Vm.Tags + vmRef := machineScope.GetVmRef() + vmRef.ResourceMap = make(map[string]string) + vmRef.ResourceMap[vmName] = vmId volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" volumeId := "vol-" + volumeName @@ -2306,6 +2348,7 @@ func TestReconcileVm(t *testing.T) { privateIps = append(privateIps, privateIp) } + // Populate SecurityGroupRef var securityGroupIds []string vmSecurityGroups := machineScope.GetVmSecurityGroups() securityGroupsRef := clusterScope.GetSecurityGroupsRef() @@ -2316,9 +2359,8 @@ func TestReconcileVm(t *testing.T) { securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId securityGroupIds = append(securityGroupIds, securityGroupId) } - deviceName := vtc.machineSpec.Node.Vm.DeviceName - vmSpec := vtc.machineSpec.Node.Vm + //vmSpec := vtc.machineSpec.Node.Vm var clockInsideLoop time.Duration = 20 var clockLoop time.Duration = 240 loadBalancerName := vtc.machineSpec.Node.Vm.LoadBalancerName @@ -2355,6 +2397,7 @@ func TestReconcileVm(t *testing.T) { ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). Return(nil, vtc.expReadTagErr) } + linkPublicIp := osc.LinkPublicIpResponse{ LinkPublicIpId: &linkPublicIpId, } @@ -2483,7 +2526,7 @@ func TestReconcileVm(t *testing.T) { }) } } - +/* // TestReconcileVmCreate has several tests to cover the code of the function reconcileVm func TestReconcileVmCreate(t *testing.T) { vmTestCases := []struct { @@ -2602,71 +2645,131 @@ func TestReconcileVmCreate(t *testing.T) { } } -// TestReconcileVmLink has several tests to cover the code of the function reconcileVm -func TestReconcileVmLink(t *testing.T) { +// TestReconcileVmGet has several tests to cover the code of the function reconcileVm +func TestReconcileVmGet(t *testing.T) { vmTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - machineSpec infrastructurev1beta1.OscMachineSpec - expCreateVmFound bool - expLinkVolumeFound bool - expCheckVmStateBootFound bool - expCheckVolumeStateAvailableFound bool - expTagFound bool - expCreateVmErr error - expReconcileVmErr error - expCheckVmStateBootErr error - expCheckVolumeStateAvailableErr error - expReadTagErr error - expLinkVolumeErr error + name string + clusterSpec infrastructurev1beta1.OscClusterSpec + machineSpec infrastructurev1beta1.OscMachineSpec + expGetVmFound bool + expGetVmStateFound bool + expAddCcmTagFound bool + expPrivateDnsNameFound bool + expPrivateIpFound bool + expTagFound bool + expGetVmStateErr error + expGetVmErr error + expAddCcmTagErr error + expPrivateDnsNameErr error + expPrivateIpErr error + expReadTagErr error + expReconcileVmErr error }{ { - name: "failed to link volume with vm", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmVolumeInitialize, - expCreateVmFound: true, - expCreateVmErr: nil, - expCheckVmStateBootFound: true, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableFound: true, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeFound: true, - expTagFound: false, - expLinkVolumeErr: fmt.Errorf("LinkVolume generic error"), - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("LinkVolume generic error Can not link volume vol-test-volume-uid with vm i-test-vm-uid for OscMachine test-system/test-osc"), + name: "get vm", + clusterSpec: defaultVmClusterInitialize, + machineSpec: defaultVmReconcile, + expGetVmFound: true, + expGetVmStateFound: true, + expAddCcmTagFound: true, + expTagFound: false, + expPrivateDnsNameFound: true, + expPrivateIpFound: true, + expGetVmErr: nil, + expGetVmStateErr: nil, + expAddCcmTagErr: nil, + expPrivateDnsNameErr: nil, + expPrivateIpErr: nil, + expReadTagErr: nil, + expReconcileVmErr: nil, + }, + { + name: "failed to get vm", + clusterSpec: defaultVmClusterInitialize, + machineSpec: defaultVmReconcile, + expGetVmFound: true, + expGetVmStateFound: false, + expAddCcmTagFound: false, + expPrivateDnsNameFound: true, + expPrivateIpFound: true, + expTagFound: false, + expGetVmErr: fmt.Errorf("GetVm generic error"), + expGetVmStateErr: nil, + expAddCcmTagErr: nil, + expPrivateDnsNameErr: nil, + expPrivateIpErr: nil, + expReadTagErr: nil, + expReconcileVmErr: fmt.Errorf("GetVm generic error"), + }, + { + name: "failed to set vmstate", + clusterSpec: defaultVmClusterInitialize, + machineSpec: defaultVmReconcile, + expGetVmFound: true, + expGetVmStateFound: true, + expAddCcmTagFound: true, + expPrivateDnsNameFound: true, + expPrivateIpFound: true, + expTagFound: false, + expGetVmErr: nil, + expAddCcmTagErr: nil, + expGetVmStateErr: fmt.Errorf("GetVmState generic error"), + expPrivateDnsNameErr: nil, + expPrivateIpErr: nil, + expReadTagErr: nil, + expReconcileVmErr: fmt.Errorf("GetVmState generic error Can not get vm i-test-vm-uid state for OscMachine test-system/test-osc"), + }, + { + name: "failed to add tag", + clusterSpec: defaultVmClusterReconcile, + machineSpec: defaultVmReconcile, + expGetVmFound: true, + expGetVmStateFound: false, + expAddCcmTagFound: true, + expPrivateDnsNameFound: true, + expPrivateIpFound: true, + expTagFound: false, + expGetVmErr: nil, + expGetVmStateErr: nil, + expAddCcmTagErr: fmt.Errorf("AddCcmTag generic error"), + expPrivateDnsNameErr: nil, + expPrivateIpErr: nil, + expReadTagErr: nil, + expReconcileVmErr: fmt.Errorf("AddCcmTag generic error can not add ccm tag test-system/test-osc"), }, { - name: "failed check vm state boot", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmVolumeInitialize, - expCreateVmFound: true, - expCreateVmErr: nil, - expCheckVmStateBootFound: true, - expCheckVmStateBootErr: fmt.Errorf("checkVmState generic error"), - expCheckVolumeStateAvailableFound: false, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeFound: false, - expLinkVolumeErr: nil, - expTagFound: false, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("checkVmState generic error Can not get vm i-test-vm-uid running for OscMachine test-system/test-osc"), + name: "Failed to retrieve privateDnsName", + clusterSpec: defaultVmClusterReconcile, + machineSpec: defaultVmReconcile, + expGetVmFound: true, + expGetVmStateFound: false, + expPrivateIpFound: true, + expAddCcmTagFound: false, + expPrivateDnsNameFound: false, + expTagFound: false, + expGetVmErr: nil, + expGetVmStateErr: nil, + expAddCcmTagErr: nil, + expPrivateIpErr: nil, + expReadTagErr: nil, + expPrivateDnsNameErr: fmt.Errorf("GetPrivateDnsNameok generic error"), + expReconcileVmErr: fmt.Errorf("Can not found privateDnsName test-system/test-osc"), }, { - name: "failed check volume state boot", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmVolumeInitialize, - expCreateVmFound: true, - expCreateVmErr: nil, - expCheckVmStateBootFound: true, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableFound: true, - expCheckVolumeStateAvailableErr: fmt.Errorf("checkVolumeState generic error"), - expLinkVolumeFound: false, - expTagFound: false, - expLinkVolumeErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("checkVolumeState generic error Can not get volume vol-test-volume-uid available for OscMachine test-system/test-osc"), + name: "Failed to retrieve privateIp", + clusterSpec: defaultVmClusterReconcile, + machineSpec: defaultVmReconcile, + expGetVmFound: true, + expGetVmStateFound: false, + expPrivateIpFound: false, + expAddCcmTagFound: false, + expPrivateDnsNameFound: true, + expGetVmErr: nil, + expGetVmStateErr: nil, + expAddCcmTagErr: nil, + expPrivateIpErr: fmt.Errorf("GetPrivateIpOk generic error"), + expPrivateDnsNameErr: nil, + expReconcileVmErr: fmt.Errorf("Can not found privateIp test-system/test-osc"), }, } for _, vtc := range vmTestCases { @@ -2674,13 +2777,27 @@ func TestReconcileVmLink(t *testing.T) { clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) vmName := vtc.machineSpec.Node.Vm.Name + "-uid" vmId := "i-" + vmName + vmState := "running" + tag := osc.Tag{ + ResourceId: &vmId, + } + if vtc.expTagFound { + mockOscTagInterface. + EXPECT(). + ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). + Return(&tag, vtc.expReadTagErr) + } else { + mockOscTagInterface. + EXPECT(). + ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). + Return(nil, vtc.expReadTagErr) + } volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" volumeId := "vol-" + volumeName volumeRef := machineScope.GetVolumeRef() volumeRef.ResourceMap = make(map[string]string) volumeRef.ResourceMap[volumeName] = volumeId - volumeStateAvailable := "available" subnetName := vtc.machineSpec.Node.Vm.SubnetName + "-uid" subnetId := "subnet-" + subnetName @@ -2716,61 +2833,59 @@ func TestReconcileVmLink(t *testing.T) { securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId securityGroupIds = append(securityGroupIds, securityGroupId) } - - vmSpec := vtc.machineSpec.Node.Vm - createVms := osc.CreateVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, + var privateDnsName string + var privateIp string + var readVms osc.ReadVmsResponse + if vtc.expPrivateDnsNameFound { + privateDnsName = "ip-0-0-0-0.eu-west-2.compute.internal" + readVms = osc.ReadVmsResponse{ + Vms: &[]osc.Vm{ + { + VmId: &vmId, + PrivateDnsName: &privateDnsName, + }, }, - }, - } - createVm := *createVms.Vms - vm := &createVm[0] - tag := osc.Tag{ - ResourceId: &vmId, - } - if vtc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(&tag, vtc.expReadTagErr) + } } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(nil, vtc.expReadTagErr) + readVms = osc.ReadVmsResponse{ + Vms: &[]osc.Vm{ + { + VmId: &vmId, + }, + }, + } } - volumeDeviceName := vtc.machineSpec.Node.Vm.VolumeDeviceName - - var clockInsideLoop time.Duration = 20 - var clockLoop time.Duration = 240 - if vtc.expCreateVmFound { + readVm := *readVms.Vms + vm := &readVm[0] + privateIp = "0.0.0.0" + if vtc.expPrivateIpFound { + vm.PrivateIp = &privateIp + } + if vtc.expGetVmFound { mockOscVmInterface. EXPECT(). - CreateVm(gomock.Eq(machineScope), gomock.Eq(&vmSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(vmName), gomock.Eq(vtc.machineSpec.Node.Vm.Tags)). - Return(vm, vtc.expCreateVmErr) + GetVm(gomock.Eq(vmId)). + Return(vm, vtc.expGetVmErr) } else { mockOscVmInterface. EXPECT(). - CreateVm(gomock.Eq(machineScope), gomock.Eq(vmSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(vmName), gomock.Eq(vtc.machineSpec.Node.Vm.Tags)). - Return(nil, vtc.expCreateVmErr) + GetVm(gomock.Eq(vmId)). + Return(nil, vtc.expGetVmErr) } - if vtc.expCheckVolumeStateAvailableFound { - mockOscVolumeInterface. + if vtc.expGetVmStateFound { + mockOscVmInterface. EXPECT(). - CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateAvailable), gomock.Eq(volumeId)). - Return(vtc.expCheckVolumeStateAvailableErr) - + GetVmState(gomock.Eq(vmId)). + Return(vmState, vtc.expGetVmStateErr) } - if vtc.expLinkVolumeFound { - mockOscVolumeInterface. + clusterName := vtc.clusterSpec.Network.Net.ClusterName + "-uid" + if vtc.expAddCcmTagFound { + mockOscVmInterface. EXPECT(). - LinkVolume(gomock.Eq(volumeId), gomock.Eq(vmId), gomock.Eq(volumeDeviceName)). - Return(vtc.expLinkVolumeErr) + AddCcmTag(gomock.Eq(clusterName), gomock.Eq(privateDnsName), gomock.Eq(vmId)). + Return(vtc.expAddCcmTagErr) } - reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface) if err != nil { assert.Equal(t, vtc.expReconcileVmErr.Error(), err.Error(), "reconcileVm() should return the same error") @@ -2778,872 +2893,11 @@ func TestReconcileVmLink(t *testing.T) { assert.Nil(t, vtc.expReconcileVmErr) } t.Logf("find reconcileVm %v\n", reconcileVm) + }) } } - -// TestReconcileVmLinkPubicIp has several tests to cover the code of the function reconcileVm -func TestReconcileVmLinkPubicIp(t *testing.T) { - vmTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - machineSpec infrastructurev1beta1.OscMachineSpec - expCreateVmFound bool - expCheckVolumeStateUseFound bool - expCheckVmStateVolumeFound bool - expCheckVmStatePublicIpFound bool - expLinkPublicIpFound bool - expTagFound bool - expCreateVmErr error - expReconcileVmErr error - expCheckVmStateBootErr error - expCheckVolumeStateAvailableErr error - expLinkVolumeErr error - expCheckVolumeStateUseErr error - expCheckVmStateVolumeErr error - expLinkPublicIpErr error - expCheckVmStatePublicIpErr error - expReadTagErr error - }{ - { - name: "failed linkPublicIp", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCheckVolumeStateUseFound: false, - expCheckVmStateVolumeFound: true, - expCheckVmStatePublicIpFound: false, - expTagFound: false, - expCreateVmErr: nil, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeErr: nil, - expCheckVolumeStateUseErr: nil, - expCheckVmStateVolumeErr: nil, - expLinkPublicIpErr: fmt.Errorf("linkPublicIp generic error"), - expCheckVmStatePublicIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("linkPublicIp generic error Can not link publicIp eipalloc-test-publicip-uid with i-test-vm-uid for OscCluster test-system/test-osc"), - }, - { - name: "failed VmStatePublicIp", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCheckVolumeStateUseFound: false, - expCheckVmStateVolumeFound: true, - expCheckVmStatePublicIpFound: true, - expTagFound: false, - expCreateVmErr: nil, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeErr: nil, - expCheckVolumeStateUseErr: nil, - expCheckVmStateVolumeErr: nil, - expLinkPublicIpErr: nil, - expCheckVmStatePublicIpErr: fmt.Errorf("CheckVmState generic error"), - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("CheckVmState generic error Can not get vm i-test-vm-uid running for OscMachine test-system/test-osc"), - }, - { - name: "failed VolumeStateUse", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmVolumeInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: false, - expCheckVolumeStateUseFound: true, - expCheckVmStateVolumeFound: false, - expCheckVmStatePublicIpFound: false, - expTagFound: false, - expCreateVmErr: nil, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeErr: nil, - expCheckVolumeStateUseErr: fmt.Errorf("CheckVolumeState generic error"), - expCheckVmStateVolumeErr: nil, - expLinkPublicIpErr: nil, - expCheckVmStatePublicIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("CheckVolumeState generic error Can not get volume vol-test-volume-uid in use for OscMachine test-system/test-osc"), - }, - { - name: "failed VmStateVolume", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: false, - expCheckVolumeStateUseFound: false, - expCheckVmStateVolumeFound: true, - expCheckVmStatePublicIpFound: false, - expTagFound: false, - expCreateVmErr: nil, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeErr: nil, - expCheckVolumeStateUseErr: nil, - expCheckVmStateVolumeErr: fmt.Errorf("CheckVmState generic error"), - expLinkPublicIpErr: nil, - expCheckVmStatePublicIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("CheckVmState generic error Can not get vm i-test-vm-uid running for OscMachine test-system/test-osc"), - }, - } - for _, vtc := range vmTestCases { - t.Run(vtc.name, func(t *testing.T) { - clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) - vmName := vtc.machineSpec.Node.Vm.Name + "-uid" - vmId := "i-" + vmName - - volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" - volumeId := "vol-" + volumeName - volumeRef := machineScope.GetVolumeRef() - volumeRef.ResourceMap = make(map[string]string) - volumeRef.ResourceMap[volumeName] = volumeId - volumeStateAvailable := "available" - volumeStateUse := "in-use" - - subnetName := vtc.machineSpec.Node.Vm.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - subnetRef.ResourceMap[subnetName] = subnetId - - publicIpName := vtc.machineSpec.Node.Vm.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - publicIpRef.ResourceMap[publicIpName] = publicIpId - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := machineScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - linkPublicIpRef.ResourceMap[vmName] = linkPublicIpId - - var privateIps []string - vmPrivateIps := machineScope.GetVmPrivateIps() - - for _, vmPrivateIp := range *vmPrivateIps { - privateIp := vmPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - var securityGroupIds []string - vmSecurityGroups := machineScope.GetVmSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, vmSecurityGroup := range *vmSecurityGroups { - securityGroupName := vmSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - - vmSpec := vtc.machineSpec.Node.Vm - var clockInsideLoop time.Duration = 20 - var clockLoop time.Duration = 240 - - createVms := osc.CreateVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - - createVm := *createVms.Vms - vm := &createVm[0] - tag := osc.Tag{ - ResourceId: &vmId, - } - if vtc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(&tag, vtc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(nil, vtc.expReadTagErr) - } - volumeDeviceName := vtc.machineSpec.Node.Vm.VolumeDeviceName - - if vtc.expCreateVmFound { - mockOscVmInterface. - EXPECT(). - CreateVm(gomock.Eq(machineScope), gomock.Eq(&vmSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(vmName), gomock.Eq(vtc.machineSpec.Node.Vm.Tags)). - Return(vm, vtc.expCreateVmErr) - } else { - mockOscVmInterface. - EXPECT(). - CreateVm(gomock.Eq(machineScope), gomock.Eq(vmSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(vmName), gomock.Eq(vtc.machineSpec.Node.Vm.Tags)). - Return(nil, vtc.expCreateVmErr) - } - - if vtc.machineSpec.Node.Vm.VolumeName != "" { - mockOscVolumeInterface. - EXPECT(). - CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateAvailable), gomock.Eq(volumeId)). - Return(vtc.expCheckVolumeStateAvailableErr) - - mockOscVolumeInterface. - EXPECT(). - LinkVolume(gomock.Eq(volumeId), gomock.Eq(vmId), gomock.Eq(volumeDeviceName)). - Return(vtc.expLinkVolumeErr) - - } - if vtc.expCheckVolumeStateUseFound { - mockOscVolumeInterface. - EXPECT(). - CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateUse), gomock.Eq(volumeId)). - Return(vtc.expCheckVolumeStateUseErr) - } - - if vtc.expLinkPublicIpFound { - mockOscPublicIpInterface. - EXPECT(). - LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). - Return("", vtc.expLinkPublicIpErr) - } - - reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, vtc.expReconcileVmErr.Error(), err.Error(), "reconcileVm() should return the same error") - } else { - assert.Nil(t, vtc.expReconcileVmErr) - } - t.Logf("find reconcileVm %v\n", reconcileVm) - }) - } -} - -// TestReconcileVmSecurityGroup has several tests to cover the code of the function reconcileVm -func TestReconcileVmSecurityGroup(t *testing.T) { - vmTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - machineSpec infrastructurev1beta1.OscMachineSpec - expCreateVmFound bool - expLinkPublicIpFound bool - expCreateSecurityGroupRuleFound bool - expTagFound bool - expCreateVmErr error - expReconcileVmErr error - expCheckVmStateBootErr error - expCheckVolumeStateAvailableErr error - expLinkVolumeErr error - expCheckVolumeStateUseErr error - expCheckVmStateVolumeErr error - expCreateSecurityGroupRuleErr error - expLinkPublicIpErr error - expCheckVmStatePublicIpErr error - expReadTagErr error - expLinkLoadBalancerBackendMachineErr error - }{ - { - name: "failed to link LoadBalancerBackendMachine ", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expCreateVmFound: true, - expLinkPublicIpFound: true, - expCreateSecurityGroupRuleFound: false, - expTagFound: false, - expCreateVmErr: nil, - expCheckVmStateBootErr: nil, - expCheckVolumeStateAvailableErr: nil, - expLinkVolumeErr: nil, - expCheckVolumeStateUseErr: nil, - expCheckVmStateVolumeErr: nil, - expLinkPublicIpErr: nil, - expCheckVmStatePublicIpErr: nil, - expLinkLoadBalancerBackendMachineErr: fmt.Errorf("LinkLoadBalancerBackendMachine generic error"), - expCreateSecurityGroupRuleErr: nil, - expReconcileVmErr: fmt.Errorf("LinkLoadBalancerBackendMachine generic error Can not link vm test-loadbalancer with loadBalancerName i-test-vm-uid for OscCluster test-system/test-osc"), - }, - } - for _, vtc := range vmTestCases { - t.Run(vtc.name, func(t *testing.T) { - clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) - vmName := vtc.machineSpec.Node.Vm.Name + "-uid" - vmId := "i-" + vmName - - tag := osc.Tag{ - ResourceId: &vmId, - } - if vtc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(&tag, vtc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(nil, vtc.expReadTagErr) - } - - volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" - volumeId := "vol-" + volumeName - volumeRef := machineScope.GetVolumeRef() - volumeRef.ResourceMap = make(map[string]string) - volumeRef.ResourceMap[volumeName] = volumeId - volumeStateAvailable := "available" - volumeStateUse := "in-use" - - subnetName := vtc.machineSpec.Node.Vm.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - subnetRef.ResourceMap[subnetName] = subnetId - - publicIpName := vtc.machineSpec.Node.Vm.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - publicIpRef.ResourceMap[publicIpName] = publicIpId - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := machineScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - linkPublicIpRef.ResourceMap[vmName] = linkPublicIpId - - var privateIps []string - vmPrivateIps := machineScope.GetVmPrivateIps() - for _, vmPrivateIp := range *vmPrivateIps { - privateIp := vmPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - var securityGroupIds []string - vmSecurityGroups := machineScope.GetVmSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, vmSecurityGroup := range *vmSecurityGroups { - securityGroupName := vmSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - - deviceName := vtc.machineSpec.Node.Vm.DeviceName - vmSpec := vtc.machineSpec.Node.Vm - var clockInsideLoop time.Duration = 20 - var clockLoop time.Duration = 240 - loadBalancerName := vtc.machineSpec.Node.Vm.LoadBalancerName - - createVms := osc.CreateVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - - createVm := *createVms.Vms - linkPublicIp := osc.LinkPublicIpResponse{ - LinkPublicIpId: &linkPublicIpId, - } - vm := &createVm[0] - if vtc.expCreateVmFound { - mockOscVmInterface. - EXPECT(). - CreateVm(gomock.Eq(machineScope), gomock.Eq(&vmSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(vmName), gomock.Eq(vtc.machineSpec.Node.Vm.Tags)). - Return(vm, vtc.expCreateVmErr) - } else { - mockOscVmInterface. - EXPECT(). - CreateVm(gomock.Eq(machineScope), gomock.Eq(vmSpec), gomock.Eq(subnetId), gomock.Eq(securityGroupIds), gomock.Eq(privateIps), gomock.Eq(vmName), gomock.Eq(vtc.machineSpec.Node.Vm.Tags)). - Return(nil, vtc.expCreateVmErr) - } - - if vtc.machineSpec.Node.Vm.VolumeName != "" { - - mockOscVolumeInterface. - EXPECT(). - CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateAvailable), gomock.Eq(volumeId)). - Return(vtc.expCheckVolumeStateAvailableErr) - - mockOscVolumeInterface. - EXPECT(). - LinkVolume(gomock.Eq(volumeId), gomock.Eq(vmId), gomock.Eq(deviceName)). - Return(vtc.expLinkVolumeErr) - - mockOscVolumeInterface. - EXPECT(). - CheckVolumeState(gomock.Eq(clockInsideLoop), gomock.Eq(clockLoop), gomock.Eq(volumeStateUse), gomock.Eq(volumeId)). - Return(vtc.expCheckVolumeStateUseErr) - } - - if vtc.expLinkPublicIpFound { - mockOscPublicIpInterface. - EXPECT(). - LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). - Return(*linkPublicIp.LinkPublicIpId, vtc.expLinkPublicIpErr) - } else { - mockOscPublicIpInterface. - EXPECT(). - LinkPublicIp(gomock.Eq(publicIpId), gomock.Eq(vmId)). - Return("", vtc.expLinkPublicIpErr) - } - - vmIds := []string{vmId} - mockOscLoadBalancerInterface. - EXPECT(). - LinkLoadBalancerBackendMachines(gomock.Eq(vmIds), gomock.Eq(loadBalancerName)). - Return(vtc.expLinkLoadBalancerBackendMachineErr) - - reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, vtc.expReconcileVmErr.Error(), err.Error(), "reconcileVm() should return the same error") - } else { - assert.Nil(t, vtc.expReconcileVmErr) - } - t.Logf("find reconcileVm %v\n", reconcileVm) - }) - } -} - -// TestReconcileVmGet has several tests to cover the code of the function reconcileVm -func TestReconcileVmGet(t *testing.T) { - vmTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - machineSpec infrastructurev1beta1.OscMachineSpec - expGetVmFound bool - expGetVmStateFound bool - expAddCcmTagFound bool - expPrivateDnsNameFound bool - expPrivateIpFound bool - expTagFound bool - expGetVmStateErr error - expGetVmErr error - expAddCcmTagErr error - expPrivateDnsNameErr error - expPrivateIpErr error - expReadTagErr error - expReconcileVmErr error - }{ - { - name: "get vm", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmReconcile, - expGetVmFound: true, - expGetVmStateFound: true, - expAddCcmTagFound: true, - expTagFound: false, - expPrivateDnsNameFound: true, - expPrivateIpFound: true, - expGetVmErr: nil, - expGetVmStateErr: nil, - expAddCcmTagErr: nil, - expPrivateDnsNameErr: nil, - expPrivateIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: nil, - }, - { - name: "failed to get vm", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmReconcile, - expGetVmFound: true, - expGetVmStateFound: false, - expAddCcmTagFound: false, - expPrivateDnsNameFound: true, - expPrivateIpFound: true, - expTagFound: false, - expGetVmErr: fmt.Errorf("GetVm generic error"), - expGetVmStateErr: nil, - expAddCcmTagErr: nil, - expPrivateDnsNameErr: nil, - expPrivateIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("GetVm generic error"), - }, - { - name: "failed to set vmstate", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmReconcile, - expGetVmFound: true, - expGetVmStateFound: true, - expAddCcmTagFound: true, - expPrivateDnsNameFound: true, - expPrivateIpFound: true, - expTagFound: false, - expGetVmErr: nil, - expAddCcmTagErr: nil, - expGetVmStateErr: fmt.Errorf("GetVmState generic error"), - expPrivateDnsNameErr: nil, - expPrivateIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("GetVmState generic error Can not get vm i-test-vm-uid state for OscMachine test-system/test-osc"), - }, - { - name: "failed to add tag", - clusterSpec: defaultVmClusterReconcile, - machineSpec: defaultVmReconcile, - expGetVmFound: true, - expGetVmStateFound: false, - expAddCcmTagFound: true, - expPrivateDnsNameFound: true, - expPrivateIpFound: true, - expTagFound: false, - expGetVmErr: nil, - expGetVmStateErr: nil, - expAddCcmTagErr: fmt.Errorf("AddCcmTag generic error"), - expPrivateDnsNameErr: nil, - expPrivateIpErr: nil, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("AddCcmTag generic error can not add ccm tag test-system/test-osc"), - }, - { - name: "Failed to retrieve privateDnsName", - clusterSpec: defaultVmClusterReconcile, - machineSpec: defaultVmReconcile, - expGetVmFound: true, - expGetVmStateFound: false, - expPrivateIpFound: true, - expAddCcmTagFound: false, - expPrivateDnsNameFound: false, - expTagFound: false, - expGetVmErr: nil, - expGetVmStateErr: nil, - expAddCcmTagErr: nil, - expPrivateIpErr: nil, - expReadTagErr: nil, - expPrivateDnsNameErr: fmt.Errorf("GetPrivateDnsNameok generic error"), - expReconcileVmErr: fmt.Errorf("Can not found privateDnsName test-system/test-osc"), - }, - { - name: "Failed to retrieve privateIp", - clusterSpec: defaultVmClusterReconcile, - machineSpec: defaultVmReconcile, - expGetVmFound: true, - expGetVmStateFound: false, - expPrivateIpFound: false, - expAddCcmTagFound: false, - expPrivateDnsNameFound: true, - expGetVmErr: nil, - expGetVmStateErr: nil, - expAddCcmTagErr: nil, - expPrivateIpErr: fmt.Errorf("GetPrivateIpOk generic error"), - expPrivateDnsNameErr: nil, - expReconcileVmErr: fmt.Errorf("Can not found privateIp test-system/test-osc"), - }, - } - for _, vtc := range vmTestCases { - t.Run(vtc.name, func(t *testing.T) { - clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) - vmName := vtc.machineSpec.Node.Vm.Name + "-uid" - vmId := "i-" + vmName - vmState := "running" - - tag := osc.Tag{ - ResourceId: &vmId, - } - if vtc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(&tag, vtc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(nil, vtc.expReadTagErr) - } - volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" - volumeId := "vol-" + volumeName - volumeRef := machineScope.GetVolumeRef() - volumeRef.ResourceMap = make(map[string]string) - volumeRef.ResourceMap[volumeName] = volumeId - - subnetName := vtc.machineSpec.Node.Vm.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - subnetRef.ResourceMap[subnetName] = subnetId - - publicIpName := vtc.machineSpec.Node.Vm.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - publicIpRef.ResourceMap[publicIpName] = publicIpId - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := machineScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - linkPublicIpRef.ResourceMap[vmName] = linkPublicIpId - - var privateIps []string - vmPrivateIps := machineScope.GetVmPrivateIps() - for _, vmPrivateIp := range *vmPrivateIps { - privateIp := vmPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - var securityGroupIds []string - vmSecurityGroups := machineScope.GetVmSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, vmSecurityGroup := range *vmSecurityGroups { - securityGroupName := vmSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - var privateDnsName string - var privateIp string - var readVms osc.ReadVmsResponse - if vtc.expPrivateDnsNameFound { - privateDnsName = "ip-0-0-0-0.eu-west-2.compute.internal" - readVms = osc.ReadVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - PrivateDnsName: &privateDnsName, - }, - }, - } - } else { - readVms = osc.ReadVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - } - readVm := *readVms.Vms - vm := &readVm[0] - privateIp = "0.0.0.0" - if vtc.expPrivateIpFound { - vm.PrivateIp = &privateIp - } - if vtc.expGetVmFound { - mockOscVmInterface. - EXPECT(). - GetVm(gomock.Eq(vmId)). - Return(vm, vtc.expGetVmErr) - } else { - mockOscVmInterface. - EXPECT(). - GetVm(gomock.Eq(vmId)). - Return(nil, vtc.expGetVmErr) - } - - if vtc.expGetVmStateFound { - mockOscVmInterface. - EXPECT(). - GetVmState(gomock.Eq(vmId)). - Return(vmState, vtc.expGetVmStateErr) - } - clusterName := vtc.clusterSpec.Network.Net.ClusterName + "-uid" - if vtc.expAddCcmTagFound { - mockOscVmInterface. - EXPECT(). - AddCcmTag(gomock.Eq(clusterName), gomock.Eq(privateDnsName), gomock.Eq(vmId)). - Return(vtc.expAddCcmTagErr) - } - reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, vtc.expReconcileVmErr.Error(), err.Error(), "reconcileVm() should return the same error") - } else { - assert.Nil(t, vtc.expReconcileVmErr) - } - t.Logf("find reconcileVm %v\n", reconcileVm) - - }) - } -} - -// TestReconcileVmResourceId has several tests to cover the code of the function reconcileVm -func TestReconcileVmResourceId(t *testing.T) { - vmTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - machineSpec infrastructurev1beta1.OscMachineSpec - expVolumeFound bool - expSubnetFound bool - expTagFound bool - expPublicIpFound bool - expLinkPublicIpFound bool - expSecurityGroupFound bool - expLoadBalancerSecurityGroupFound bool - expCreatePublicIpFound bool - expReadTagErr error - expReconcileVmErr error - }{ - { - name: "Volume does not exist ", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmVolumeInitialize, - expVolumeFound: false, - expSubnetFound: true, - expPublicIpFound: true, - expLinkPublicIpFound: true, - expSecurityGroupFound: true, - expLoadBalancerSecurityGroupFound: true, - expTagFound: false, - expCreatePublicIpFound: false, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("test-volume-uid does not exist"), - }, - { - name: "Volume does not exist ", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expVolumeFound: true, - expSubnetFound: false, - expPublicIpFound: true, - expLinkPublicIpFound: true, - expSecurityGroupFound: true, - expLoadBalancerSecurityGroupFound: true, - expTagFound: false, - expCreatePublicIpFound: false, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("test-subnet-uid does not exist"), - }, - { - name: "PublicIp does not exist on clusterScope", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expVolumeFound: true, - expSubnetFound: true, - expPublicIpFound: false, - expLinkPublicIpFound: true, - expSecurityGroupFound: true, - expLoadBalancerSecurityGroupFound: true, - expTagFound: false, - expCreatePublicIpFound: false, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("test-publicip-uid does not exist"), - }, - { - name: "SecurityGroup does not exist ", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expVolumeFound: true, - expSubnetFound: true, - expPublicIpFound: true, - expLinkPublicIpFound: true, - expSecurityGroupFound: false, - expLoadBalancerSecurityGroupFound: false, - expTagFound: false, - expCreatePublicIpFound: false, - expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("test-securitygroup-uid does not exist"), - }, - { - name: "failed to get tag", - clusterSpec: defaultVmClusterInitialize, - machineSpec: defaultVmInitialize, - expVolumeFound: true, - expSubnetFound: true, - expPublicIpFound: true, - expLinkPublicIpFound: true, - expSecurityGroupFound: true, - expLoadBalancerSecurityGroupFound: true, - expTagFound: true, - expCreatePublicIpFound: false, - expReadTagErr: fmt.Errorf("ReadTag generic error"), - expReconcileVmErr: fmt.Errorf("ReadTag generic error Can not get tag for OscMachine test-system/test-osc"), - }, - } - for _, vtc := range vmTestCases { - t.Run(vtc.name, func(t *testing.T) { - clusterScope, machineScope, ctx, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) - vmName := vtc.machineSpec.Node.Vm.Name + "-uid" - vmId := "i-" + vmName - volumeName := vtc.machineSpec.Node.Vm.VolumeName + "-uid" - volumeId := "vol-" + volumeName - volumeRef := machineScope.GetVolumeRef() - volumeRef.ResourceMap = make(map[string]string) - if vtc.expVolumeFound { - volumeRef.ResourceMap[volumeName] = volumeId - } - - subnetName := vtc.machineSpec.Node.Vm.SubnetName + "-uid" - subnetId := "subnet-" + subnetName - subnetRef := clusterScope.GetSubnetRef() - subnetRef.ResourceMap = make(map[string]string) - if vtc.expSubnetFound { - subnetRef.ResourceMap[subnetName] = subnetId - } - - publicIpName := vtc.machineSpec.Node.Vm.PublicIpName + "-uid" - publicIpId := "eipalloc-" + publicIpName - publicIpRef := clusterScope.GetPublicIpRef() - publicIpRef.ResourceMap = make(map[string]string) - if vtc.expPublicIpFound { - publicIpRef.ResourceMap[publicIpName] = publicIpId - } - - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := machineScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - if vtc.expLinkPublicIpFound { - linkPublicIpRef.ResourceMap[vmName] = linkPublicIpId - } - - var privateIps []string - vmPrivateIps := machineScope.GetVmPrivateIps() - for _, vmPrivateIp := range *vmPrivateIps { - privateIp := vmPrivateIp.PrivateIp - privateIps = append(privateIps, privateIp) - } - - tag := osc.Tag{ - ResourceId: &vmId, - } - if vtc.expVolumeFound && vtc.expSubnetFound && vtc.expPublicIpFound && vtc.expLinkPublicIpFound && vtc.expSecurityGroupFound { - if vtc.expTagFound { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(&tag, vtc.expReadTagErr) - } else { - mockOscTagInterface. - EXPECT(). - ReadTag(gomock.Eq("Name"), gomock.Eq(vmName)). - Return(nil, vtc.expReadTagErr) - } - } - - var securityGroupIds []string - vmSecurityGroups := machineScope.GetVmSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, vmSecurityGroup := range *vmSecurityGroups { - securityGroupName := vmSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - if vtc.expSecurityGroupFound { - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - } - securityGroupIds = append(securityGroupIds, securityGroupId) - } - - loadBalancerSpec := clusterScope.GetLoadBalancer() - loadBalancerSpec.SetDefaultValue() - loadBalancerSecurityGroupName := loadBalancerSpec.SecurityGroupName - loadBalancerSecurityGroupClusterScopeName := loadBalancerSecurityGroupName + "-uid" - loadBalancerSecurityGroupId := "sg-" + loadBalancerSecurityGroupClusterScopeName - if vtc.expLoadBalancerSecurityGroupFound { - securityGroupsRef.ResourceMap[loadBalancerSecurityGroupClusterScopeName] = loadBalancerSecurityGroupId - } - - reconcileVm, err := reconcileVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscVolumeInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, mockOscTagInterface) - if err != nil { - assert.Equal(t, vtc.expReconcileVmErr.Error(), err.Error(), "reconcileVm() should return the same error") - } else { - assert.Nil(t, vtc.expReconcileVmErr) - } - t.Logf("find reconcileVm %v\n", reconcileVm) - }) - } -} - +*/ // TestReconcileDeleteVm has several tests to cover the code of the function reconcileDeleteVm func TestReconcileDeleteVm(t *testing.T) { vmMachine1 := defaultVmReconcile @@ -3988,167 +3242,6 @@ func TestReconcileDeleteVmUnlinkPublicIp(t *testing.T) { } -// TestReconcileDeleteVmLoadBalancer has several tests to cover the code of the function reconcileDeleteVm -func TestReconcileDeleteVmLoadBalancer(t *testing.T) { - vmTestCases := []struct { - name string - clusterSpec infrastructurev1beta1.OscClusterSpec - machineSpec infrastructurev1beta1.OscMachineSpec - expUnlinkLoadBalancerBackendMachineErr error - expDeleteInboundSecurityGroupRuleErr error - expDeleteOutboundSecurityGroupRuleErr error - expUnlinkLoadBalancerBackendMachineFound bool - expDeleteInboundSecurityGroupRuleFound bool - expDeleteOutboundSecurityGroupRuleFound bool - expGetVmErr error - expGetVmFound bool - expCheckUnlinkPublicIpErr error - expReconcileDeleteVmErr error - }{ - { - name: "failed UnlinkLoadBalancerBackendMachineFound", - clusterSpec: defaultVmClusterReconcile, - machineSpec: defaultVmReconcile, - expUnlinkLoadBalancerBackendMachineErr: fmt.Errorf("UnlinkLoadBalancerBackendMachineFound generic error"), - expDeleteInboundSecurityGroupRuleErr: nil, - expDeleteOutboundSecurityGroupRuleErr: nil, - expDeleteInboundSecurityGroupRuleFound: false, - expDeleteOutboundSecurityGroupRuleFound: false, - expUnlinkLoadBalancerBackendMachineFound: true, - expGetVmFound: true, - expGetVmErr: nil, - expCheckUnlinkPublicIpErr: nil, - expReconcileDeleteVmErr: fmt.Errorf("UnlinkLoadBalancerBackendMachineFound generic error Can not unlink vm test-loadbalancer with loadBalancerName i-test-vm-uid for OscCluster test-system/test-osc"), - }, - { - name: "failed DeleteSecurityGroupRule", - clusterSpec: defaultVmClusterReconcile, - machineSpec: defaultVmReconcile, - expUnlinkLoadBalancerBackendMachineErr: nil, - expDeleteInboundSecurityGroupRuleErr: nil, - expDeleteOutboundSecurityGroupRuleErr: fmt.Errorf("DeleteSecurityGroupRule generic error"), - expDeleteInboundSecurityGroupRuleFound: false, - expDeleteOutboundSecurityGroupRuleFound: true, - expUnlinkLoadBalancerBackendMachineFound: true, - expGetVmFound: true, - expGetVmErr: nil, - expCheckUnlinkPublicIpErr: nil, - expReconcileDeleteVmErr: fmt.Errorf("DeleteSecurityGroupRule generic error Can not delete outbound securityGroupRule for OscCluster test-system/test-osc"), - }, - { - name: "vm is already deleted", - clusterSpec: defaultVmClusterReconcile, - machineSpec: defaultVmReconcile, - expUnlinkLoadBalancerBackendMachineErr: nil, - expDeleteInboundSecurityGroupRuleErr: nil, - expDeleteOutboundSecurityGroupRuleErr: nil, - expDeleteInboundSecurityGroupRuleFound: true, - expDeleteOutboundSecurityGroupRuleFound: true, - expUnlinkLoadBalancerBackendMachineFound: true, - expGetVmFound: false, - expGetVmErr: nil, - expCheckUnlinkPublicIpErr: nil, - expReconcileDeleteVmErr: nil, - }, - } - for _, vtc := range vmTestCases { - t.Run(vtc.name, func(t *testing.T) { - clusterScope, machineScope, ctx, mockOscVmInterface, _, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface, _ := SetupWithVmMock(t, vtc.name, vtc.clusterSpec, vtc.machineSpec) - vmName := vtc.machineSpec.Node.Vm.Name + "-uid" - vmId := "i-" + vmName - vmRef := machineScope.GetVmRef() - vmRef.ResourceMap = make(map[string]string) - if vtc.expGetVmFound { - vmRef.ResourceMap[vmName] = vmId - } - - var securityGroupIds []string - vmSecurityGroups := machineScope.GetVmSecurityGroups() - securityGroupsRef := clusterScope.GetSecurityGroupsRef() - securityGroupsRef.ResourceMap = make(map[string]string) - for _, vmSecurityGroup := range *vmSecurityGroups { - securityGroupName := vmSecurityGroup.Name + "-uid" - securityGroupId := "sg-" + securityGroupName - securityGroupsRef.ResourceMap[securityGroupName] = securityGroupId - securityGroupIds = append(securityGroupIds, securityGroupId) - } - publicIpName := vtc.machineSpec.Node.Vm.PublicIpName + "-uid" - linkPublicIpId := "eipassoc-" + publicIpName - linkPublicIpRef := machineScope.GetLinkPublicIpRef() - linkPublicIpRef.ResourceMap = make(map[string]string) - linkPublicIpRef.ResourceMap[publicIpName] = linkPublicIpId - - loadBalancerName := vtc.machineSpec.Node.Vm.LoadBalancerName - loadBalancerSpec := clusterScope.GetLoadBalancer() - loadBalancerSpec.SetDefaultValue() - loadBalancerSecurityGroupName := loadBalancerSpec.SecurityGroupName - ipProtocol := strings.ToLower(loadBalancerSpec.Listener.BackendProtocol) - fromPortRange := loadBalancerSpec.Listener.BackendPort - toPortRange := loadBalancerSpec.Listener.BackendPort - loadBalancerSecurityGroupClusterScopeName := loadBalancerSecurityGroupName + "-uid" - loadBalancerSecurityGroupId := "sg-" + loadBalancerSecurityGroupClusterScopeName - securityGroupsRef.ResourceMap[loadBalancerSecurityGroupClusterScopeName] = loadBalancerSecurityGroupId - associateSecurityGroupId := securityGroupsRef.ResourceMap[loadBalancerSecurityGroupClusterScopeName] - - createVms := osc.CreateVmsResponse{ - Vms: &[]osc.Vm{ - { - VmId: &vmId, - }, - }, - } - - createVm := *createVms.Vms - vm := &createVm[0] - if vtc.expGetVmFound { - mockOscVmInterface. - EXPECT(). - GetVm(gomock.Eq(vmId)). - Return(vm, vtc.expGetVmErr) - } else { - mockOscVmInterface. - EXPECT(). - GetVm(gomock.Eq(vmId)). - Return(nil, vtc.expGetVmErr) - } - - mockOscPublicIpInterface. - EXPECT(). - UnlinkPublicIp(gomock.Eq(linkPublicIpId)). - Return(vtc.expCheckUnlinkPublicIpErr) - vmIds := []string{vmId} - if vtc.expUnlinkLoadBalancerBackendMachineFound { - mockOscLoadBalancerInterface. - EXPECT(). - UnlinkLoadBalancerBackendMachines(gomock.Eq(vmIds), gomock.Eq(loadBalancerName)). - Return(vtc.expUnlinkLoadBalancerBackendMachineErr) - } - - if vtc.expDeleteOutboundSecurityGroupRuleFound { - mockOscSecurityGroupInterface. - EXPECT(). - DeleteSecurityGroupRule(gomock.Eq(associateSecurityGroupId), gomock.Eq("Outbound"), gomock.Eq(ipProtocol), "", gomock.Eq(securityGroupIds[0]), gomock.Eq(fromPortRange), gomock.Eq(toPortRange)). - Return(vtc.expDeleteOutboundSecurityGroupRuleErr) - } - if vtc.expDeleteInboundSecurityGroupRuleFound { - mockOscSecurityGroupInterface. - EXPECT(). - DeleteSecurityGroupRule(gomock.Eq(associateSecurityGroupId), gomock.Eq("Inbound"), gomock.Eq(ipProtocol), "", gomock.Eq(securityGroupIds[0]), gomock.Eq(fromPortRange), gomock.Eq(toPortRange)). - Return(vtc.expDeleteOutboundSecurityGroupRuleErr) - } - - reconcileDeleteVm, err := reconcileDeleteVm(ctx, clusterScope, machineScope, mockOscVmInterface, mockOscPublicIpInterface, mockOscLoadBalancerInterface, mockOscSecurityGroupInterface) - if err != nil { - assert.Equal(t, vtc.expReconcileDeleteVmErr.Error(), err.Error(), "reconcileDeleteVm() hould return the same error") - } else { - assert.Nil(t, vtc.expReconcileDeleteVmErr) - } - t.Logf("find reconcileDeleteVm %v\n", reconcileDeleteVm) - }) - } - -} - // TestReconcileDeleteVmResourceId has several tests to cover the code of the function reconcileDeleteVm func TestReconcileDeleteVmResourceId(t *testing.T) { vmTestCases := []struct { diff --git a/controllers/oscmachine_volume_controller_unit_test.go b/controllers/oscmachine_volume_controller_unit_test.go index a4b5981d4..7587bdbe3 100644 --- a/controllers/oscmachine_volume_controller_unit_test.go +++ b/controllers/oscmachine_volume_controller_unit_test.go @@ -291,6 +291,7 @@ func TestCheckVolumeOscDuplicateName(t *testing.T) { } } +/* // TestReconcileVolumeResourceId has several tests to cover the code of the function reconcileVolume func TestReconcileVolumeResourceId(t *testing.T) { vmTestCases := []struct { @@ -333,7 +334,7 @@ func TestReconcileVolumeResourceId(t *testing.T) { expLoadBalancerSecurityGroupFound: true, expTagFound: false, expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("test-subnet-uid does not exist"), + expReconcileVmErr: fmt.Errorf("failed to get subnet ID for test-subnet-uid: test-subnet-uid does not exist"), }, { name: "PublicIp does not exist ", @@ -361,7 +362,7 @@ func TestReconcileVolumeResourceId(t *testing.T) { expLoadBalancerSecurityGroupFound: false, expTagFound: false, expReadTagErr: nil, - expReconcileVmErr: fmt.Errorf("test-securitygroup-uid does not exist"), + expReconcileVmErr: fmt.Errorf("failed to handle security groups: failed to get security group ID for test-securitygroup-uid"), }, { name: "failed to get tag", @@ -469,7 +470,7 @@ func TestReconcileVolumeResourceId(t *testing.T) { t.Logf("find reconcileVm %v\n", reconcileVm) }) } -} +}*/ // TestReconcileVolumeCreate has several tests to cover the code of the function reconcileVolume func TestReconcileVolumeCreate(t *testing.T) { diff --git a/controllers/oscmachinetemplate_capacity_controller.go b/controllers/oscmachinetemplate_capacity_controller.go index 6e3ce5760..a5bc6a67e 100644 --- a/controllers/oscmachinetemplate_capacity_controller.go +++ b/controllers/oscmachinetemplate_capacity_controller.go @@ -92,7 +92,7 @@ func (r *OscMachineTemplateReconciler) Reconcile(ctx context.Context, req ctrl.R } if len(clusterList.Items) == 0 { log.V(2).Info("OscCluster is not available yet") - return reconcile.Result{RequeueAfter: 30 * time.Second}, nil + return reconcile.Result{RequeueAfter: 60 * time.Second}, nil } } oscCluster := &infrastructurev1beta1.OscCluster{} From 573d36685e84d3c24a6dc0a35d768c295e892488 Mon Sep 17 00:00:00 2001 From: Thiery Ouattara Date: Thu, 3 Oct 2024 09:17:49 +0000 Subject: [PATCH 09/11] fix go version in go mod --- .github/workflows/build.yaml | 13 ++++++++----- go.mod | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index d896c06c8..0dcc548b9 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -5,6 +5,8 @@ on: paths: - "**.go" - "**.yaml" + - "**.mod" + - "**.sum" - "!capm.yaml" - "!osc-secret.yaml" - "!example/**.yaml" @@ -37,10 +39,11 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 with: - go-version: '1.23' + go-version-file: './go.mod' - name: check-gofmt run: make checkfmt shell: bash @@ -54,6 +57,6 @@ jobs: run: pip install yamllint - name: check with yamlint run: yamllint -c .github/linters/yaml-lint.yaml . --format github - - name: Lint - run: make vet + - name: Run make build + run: make build shell: bash diff --git a/go.mod b/go.mod index 9d2722c80..927c5cca8 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/outscale-dev/cluster-api-provider-outscale.git -go 1.23 +go 1.23.0 require ( github.com/Jeffail/gabs v1.4.0 From 90e2c8820274494c405a4cf06c566a0bb80c36c0 Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Thu, 12 Dec 2024 10:08:24 +0000 Subject: [PATCH 10/11] Setting -mod=readonly ensures go modules are not inadvertently modified during builds or tests. Signed-off-by: hanenMizouni --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 34ce8024c..760b07874 100644 --- a/Makefile +++ b/Makefile @@ -36,6 +36,8 @@ CAPI_NAMESPACE ?= capi-kubeadm-bootstrap-system CAPO_NAMESPACE ?= cluster-api-provider-outscale-system # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. ENVTEST_K8S_VERSION = 1.30.3 +GOFLAGS=-mod=readonly +export GOFLAGS MINIMUM_KUBEBUILDERTOOL_VERSION=1.30.3 MINIMUM_ENVTEST_VERSION=1.30.3 E2E_CONF_FILE_SOURCE ?= ${PWD}/test/e2e/config/outscale-ci.yaml From 0933d20f2af55b884183f1ae14b17a4ec65f957c Mon Sep 17 00:00:00 2001 From: hanenMizouni Date: Thu, 12 Dec 2024 10:26:30 +0000 Subject: [PATCH 11/11] remove unecessary vendor Signed-off-by: hanenMizouni --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 760b07874..857679f4e 100644 --- a/Makefile +++ b/Makefile @@ -139,7 +139,7 @@ vet: ## Run go vet against code. format: gofmt gospace yamlspace yamlfmt gofmt: ## Run gofmt - find . -name "*.go" | grep -v "\/vendor\/" | xargs gofmt -s -w + find . -name "*.go" | xargs gofmt -s -w .PHONY: gospace gospace: ## Run to remove trailling space @@ -151,7 +151,7 @@ yamlspace: ## Run to remove trailling space .PHONY: yamlfmt yamlfmt: install-yamlfmt - find . -name "*.yaml" -not -path "./helm/*" -not -path "./.github/workflows/*" | grep -v "\/vendor\/" | xargs yamlfmt + find . -name "*.yaml" -not -path "./helm/*" -not -path "./.github/workflows/*" | xargs yamlfmt .PHONY: checkfmt checkfmt: ## check gofmt