feat(VM): Improve robustness of vm and bastion reconcilers

outscale · Sep 24, 2024 · 905e534 · 905e534
1 parent 457a867
commit 905e534
Show file tree

Hide file tree

Showing 17 changed files with 302 additions and 579 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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: