Requeue non empty vpc deletion

api/v1alpha1/linodemachine_types.go

@@ -80,38 +80,28 @@ type LinodeMachineSpec struct {
 // InstanceMetadataOptions defines metadata of instance
 type InstanceMetadataOptions struct {
 	// UserData expects a Base64-encoded string
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	UserData string `json:"userData,omitempty"`
 }
 
 // InstanceConfigInterfaceCreateOptions defines network interface config
 type InstanceConfigInterfaceCreateOptions struct {
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	IPAMAddress string `json:"ipamAddress,omitempty"`
 	// +kubebuilder:validation:MinLength=3
 	// +kubebuilder:validation:MaxLength=63
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	// +optional
-	Label string `json:"label,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	Label   string                          `json:"label,omitempty"`
 	Purpose linodego.ConfigInterfacePurpose `json:"purpose,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	Primary bool `json:"primary,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	Primary bool                            `json:"primary,omitempty"`
 	// +optional
 	SubnetID *int `json:"subnetId,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	// +optional
-	IPv4 *VPCIPv4 `json:"ipv4,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	IPv4     *VPCIPv4 `json:"ipv4,omitempty"`
 	IPRanges []string `json:"ipRanges,omitempty"`
 }
 
 // VPCIPv4 defines VPC IPV4 settings
 type VPCIPv4 struct {
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	VPC string `json:"vpc,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	VPC     string `json:"vpc,omitempty"`
 	NAT1To1 string `json:"nat1to1,omitempty"`
 }
 

api/v1alpha1/linodevpc_types.go

@@ -47,10 +47,8 @@ type LinodeVPCSpec struct {
 type VPCSubnetCreateOptions struct {
 	// +kubebuilder:validation:MinLength=3
 	// +kubebuilder:validation:MaxLength=63
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	// +optional
 	Label string `json:"label,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	// +optional
 	IPv4 string `json:"ipv4,omitempty"`
 }

config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml

@@ -74,6 +74,73 @@ spec:
                       region:
                         description: The Linode Region the LinodeCluster lives in.
                         type: string
+                      vpcRef:
+                        description: "ObjectReference contains enough information
+                          to let you inspect or modify the referred object. --- New
+                          uses of this type are discouraged because of difficulty
+                          describing its usage when embedded in APIs. 1. Ignored fields.
+                          \ It includes many fields which are not generally honored.
+                          \ For instance, ResourceVersion and FieldPath are both very
+                          rarely valid in actual usage. 2. Invalid usage help.  It
+                          is impossible to add specific help for individual usage.
+                          \ In most embedded usages, there are particular restrictions
+                          like, \"must refer only to types A and B\" or \"UID not
+                          honored\" or \"name must be restricted\". Those cannot be
+                          well described when embedded. 3. Inconsistent validation.
+                          \ Because the usages are different, the validation rules
+                          are different by usage, which makes it hard for users to
+                          predict what will happen. 4. The fields are both imprecise
+                          and overly precise.  Kind is not a precise mapping to a
+                          URL. This can produce ambiguity during interpretation and
+                          require a REST mapping.  In most cases, the dependency is
+                          on the group,resource tuple and the version of the actual
+                          struct is irrelevant. 5. We cannot easily change it.  Because
+                          this type is embedded in many locations, updates to this
+                          type will affect numerous schemas.  Don't make new APIs
+                          embed an underspecified API type they do not control. \n
+                          Instead of using this type, create a locally provided and
+                          used type that is well-focused on your reference. For example,
+                          ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533
+                          ."
+                        properties:
+                          apiVersion:
+                            description: API version of the referent.
+                            type: string
+                          fieldPath:
+                            description: 'If referring to a piece of an object instead
+                              of an entire object, this string should contain a valid
+                              JSON/Go field access statement, such as desiredState.manifest.containers[2].
+                              For example, if the object reference is to a container
+                              within a pod, this would take on a value like: "spec.containers{name}"
+                              (where "name" refers to the name of the container that
+                              triggered the event) or if no container name is specified
+                              "spec.containers[2]" (container with index 2 in this
+                              pod). This syntax is chosen only to have some well-defined
+                              way of referencing a part of an object. TODO: this design
+                              is not final and this field is subject to change in
+                              the future.'
+                            type: string
+                          kind:
+                            description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+                            type: string
+                          name:
+                            description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
+                            type: string
+                          namespace:
+                            description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
+                            type: string
+                          resourceVersion:
+                            description: 'Specific resourceVersion to which this reference
+                              is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
+                            type: string
+                          uid:
+                            description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
+                            type: string
+                        type: object
+                        x-kubernetes-map-type: atomic
+                        x-kubernetes-validations:
+                        - message: Value is immutable
+                          rule: self == oldSelf
                     required:
                     - region
                     type: object

config/crd/bases/infrastructure.cluster.x-k8s.io_linodemachines.yaml

@@ -85,55 +85,28 @@ spec:
                       items:
                         type: string
                       type: array
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     ipamAddress:
                       type: string
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     ipv4:
                       description: VPCIPv4 defines VPC IPV4 settings
                       properties:
                         nat1to1:
                           type: string
-                          x-kubernetes-validations:
-                          - message: Value is immutable
-                            rule: self == oldSelf
                         vpc:
                           type: string
-                          x-kubernetes-validations:
-                          - message: Value is immutable
-                            rule: self == oldSelf
                       type: object
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     label:
                       maxLength: 63
                       minLength: 3
                       type: string
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     primary:
                       type: boolean
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     purpose:
                       description: ConfigInterfacePurpose options start with InterfacePurpose
                         and include all known interface purpose types
                       type: string
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     subnetId:
                       type: integer
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                   type: object
                 type: array
                 x-kubernetes-validations:
@@ -152,9 +125,6 @@ spec:
                   userData:
                     description: UserData expects a Base64-encoded string
                     type: string
-                    x-kubernetes-validations:
-                    - message: Value is immutable
-                      rule: self == oldSelf
                 type: object
                 x-kubernetes-validations:
                 - message: Value is immutable

config/crd/bases/infrastructure.cluster.x-k8s.io_linodemachinetemplates.yaml

@@ -109,6 +109,8 @@ spec:
                                   type: string
                               type: object
                             label:
+                              maxLength: 63
+                              minLength: 3
                               type: string
                             primary:
                               type: boolean
@@ -125,6 +127,8 @@ spec:
                         - message: Value is immutable
                           rule: self == oldSelf
                       label:
+                        maxLength: 63
+                        minLength: 3
                         type: string
                         x-kubernetes-validations:
                         - message: Value is immutable

config/crd/bases/infrastructure.cluster.x-k8s.io_linodevpcs.yaml

@@ -54,16 +54,10 @@ spec:
                   properties:
                     ipv4:
                       type: string
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                     label:
                       maxLength: 63
                       minLength: 3
                       type: string
-                      x-kubernetes-validations:
-                      - message: Value is immutable
-                        rule: self == oldSelf
                   type: object
                 type: array
                 x-kubernetes-validations:

controller/linodemachine_controller.go

@@ -54,12 +54,12 @@ var skippedMachinePhases = map[string]bool{
 }
 
 var skippedInstanceStatuses = map[linodego.InstanceStatus]bool{
-	linodego.InstanceOffline:      true,
 	linodego.InstanceShuttingDown: true,
 	linodego.InstanceDeleting:     true,
 }
 
 var requeueInstanceStatuses = map[linodego.InstanceStatus]bool{
+	linodego.InstanceOffline:      true,
 	linodego.InstanceBooting:      true,
 	linodego.InstanceRebooting:    true,
 	linodego.InstanceProvisioning: true,
@@ -154,7 +154,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		Name:      cluster.Spec.InfrastructureRef.Name,
 	}
 
-	if err := r.Client.Get(ctx, linodeClusterKey, linodeCluster); err != nil {
+	if err = r.Client.Get(ctx, linodeClusterKey, linodeCluster); err != nil {
 		log.Error(err, "Failed to fetch Linode cluster")
 
 		return ctrl.Result{}, client.IgnoreNotFound(err)
@@ -248,6 +248,8 @@ func (r *LinodeMachineReconciler) reconcile(
 }
 
 func (r *LinodeMachineReconciler) reconcileCreate(ctx context.Context, machineScope *scope.MachineScope, logger logr.Logger) (*linodego.Instance, error) {
+	logger.Info("creating machine")
+
 	tags := []string{string(machineScope.LinodeCluster.UID), string(machineScope.LinodeMachine.UID)}
 
 	linodeInstances, err := machineScope.LinodeClient.ListInstances(ctx, linodego.NewListOptions(1, util.CreateLinodeAPIFilter("", tags)))
@@ -327,21 +329,20 @@ func (r *LinodeMachineReconciler) reconcileCreate(ctx context.Context, machineSc
 }
 
 func (r *LinodeMachineReconciler) reconcileUpdate(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (res reconcile.Result, linodeInstance *linodego.Instance, err error) {
-	if machineScope.LinodeMachine.Spec.InstanceID == nil {
-		err = errors.New("missing instance ID")
-
-		return
-	}
+	logger.Info("updating machine")
 
 	res = ctrl.Result{}
 
-	if linodeInstance, err = machineScope.LinodeClient.GetInstance(ctx, *machineScope.LinodeMachine.Spec.InstanceID); err != nil {
-		logger.Error(err, "Failed to get Linode machine instance")
+	if machineScope.LinodeMachine.Spec.InstanceID == nil {
+		return res, nil, errors.New("missing instance ID")
+	}
 
-		// Not found is not an error
-		apiErr := linodego.Error{Code: http.StatusNotFound}
-		if apiErr.Is(err) {
-			err = nil
+	if linodeInstance, err = machineScope.LinodeClient.GetInstance(ctx, *machineScope.LinodeMachine.Spec.InstanceID); err != nil {
+		err = util.IgnoreLinodeAPIError(err, http.StatusNotFound)
+		if err != nil {
+			logger.Error(err, "Failed to get Linode machine instance")
+		} else {
+			logger.Info("Instance not found, let's create a new one")
 
 			// Create new machine
 			machineScope.LinodeMachine.Spec.ProviderID = nil
@@ -350,44 +351,46 @@ func (r *LinodeMachineReconciler) reconcileUpdate(ctx context.Context, logger lo
 			conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, string("missing"), clusterv1.ConditionSeverityWarning, "instance not found")
 		}
 
-		return
+		return res, linodeInstance, err
 	}
 
 	if _, ok := requeueInstanceStatuses[linodeInstance.Status]; ok {
 		if linodeInstance.Updated.Add(reconciler.DefaultMachineControllerWaitForRunningTimeout).After(time.Now()) {
 			logger.Info("Instance has one operaton running, re-queuing reconciliation", "status", linodeInstance.Status)
 
-			res = ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForRunningDelay}
-		} else {
-			logger.Info("Instance has one operaton long running, skipping reconciliation", "status", linodeInstance.Status)
+			return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForRunningDelay}, linodeInstance, nil
 		}
 
-		return
+		logger.Info("Instance has one operaton long running, skipping reconciliation", "status", linodeInstance.Status)
+
+		conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, string(linodeInstance.Status), clusterv1.ConditionSeverityInfo, "skipped due to long running operation")
+
+		return res, linodeInstance, nil
 	} else if _, ok := skippedInstanceStatuses[linodeInstance.Status]; ok || linodeInstance.Status != linodego.InstanceRunning {
 		logger.Info("Instance has incompatible status, skipping reconciliation", "status", linodeInstance.Status)
 
 		conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, string(linodeInstance.Status), clusterv1.ConditionSeverityInfo, "incompatible status")
 
-		return
+		return res, linodeInstance, nil
 	}
 
+	machineScope.LinodeMachine.Status.Ready = true
+
 	conditions.MarkTrue(machineScope.LinodeMachine, clusterv1.ReadyCondition)
 
 	r.Recorder.Event(machineScope.LinodeMachine, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "instance is running")
 
-	return
+	return res, linodeInstance, nil
 }
 
 func (r *LinodeMachineReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) error {
 	logger.Info("deleting machine")
 
 	if machineScope.LinodeMachine.Spec.InstanceID != nil {
 		if err := machineScope.LinodeClient.DeleteInstance(ctx, *machineScope.LinodeMachine.Spec.InstanceID); err != nil {
-			logger.Info("Failed to delete Linode machine instance")
+			if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
+				logger.Error(err, "Failed to delete Linode machine instance")
 
-			// Not found is not an error
-			apiErr := linodego.Error{Code: http.StatusNotFound}
-			if !apiErr.Is(err) {
 				return err
 			}
 		}