fix: workload nil pointer deref (#73)

* fix: add check for the nil pointers in the wait methods

* fix: double limits

* tests: conditions

* return error as just a string and log it

* release: 1.4.6

Makefile

@@ -1,6 +1,6 @@
 #include tests/e2e/Makefile
 
-VERSION ?= 1.4.5
+VERSION ?= 1.4.6
 
 # check if we are using MacOS or LINUX and use that to determine the sed command
 UNAME_S := $(shell uname -s)

chart/Chart.yaml

@@ -5,12 +5,12 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.4.5
+version: 1.4.6
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
 # It is recommended to use it with quotes.
-appVersion: "v1.4.5"
+appVersion: "v1.4.6"
 dependencies:
   - name: common
     repository: https://charts.bitnami.com/bitnami

chart/values.yaml

@@ -4,7 +4,7 @@
 
 image:
   repository: docker.io/uffizzi/uffizzi-cluster-operator
-  tag: v1.4.5
+  tag: v1.4.6
 # `flux` dependency values
 flux:
   helmController:

controllers/helm/build/vcluster/build.go

@@ -268,25 +268,25 @@ func isolation() vcluster.Isolation {
 				RequestsMemory:              "10Gi",
 				RequestsEphemeralStorage:    "15Gi",
 				RequestsStorage:             "10Gi",
-				LimitsCpu:                   "10",
-				LimitsMemory:                "15Gi",
-				LimitsEphemeralStorage:      "30Gi",
-				ServicesLoadbalancers:       5,
+				LimitsCpu:                   "20",
+				LimitsMemory:                "30Gi",
+				LimitsEphemeralStorage:      "60Gi",
+				ServicesLoadbalancers:       100,
 				ServicesNodePorts:           0,
-				CountEndpoints:              40,
+				CountEndpoints:              100,
 				CountConfigmaps:             100,
-				CountPersistentVolumeClaims: 40,
-				CountPods:                   40,
+				CountPersistentVolumeClaims: 100,
+				CountPods:                   100,
 				CountSecrets:                100,
-				CountServices:               40,
+				CountServices:               100,
 			},
 		},
 		LimitRange: vcluster.LimitRange{
 			Enabled: true,
 			Default: vcluster.LimitRangeResources{
 				Cpu:              "1",
-				Memory:           "512Mi",
-				EphemeralStorage: "8Gi",
+				Memory:           "1Gi",
+				EphemeralStorage: "16Gi",
 			},
 			DefaultRequest: vcluster.LimitRangeResources{
 				Cpu:              "100m",

controllers/helm/types/vcluster/vcluster.go

@@ -67,13 +67,13 @@ type ContainerVolumeMounts struct {
 	MountPath string `json:"mountPath,omitempty"`
 }
 
-// VClusterInit - resources which are created during the init phase of the vcluster
+// Init - resources which are created during the init phase of the vcluster
 type Init struct {
 	Manifests string               `json:"manifests,omitempty"`
 	Helm      []v1alpha1.HelmChart `json:"helm,omitempty"`
 }
 
-// VClusterSyncer - parameters to create the syncer with
+// Syncer - parameters to create the syncer with
 // https://www.vcluster.com/docs/architecture/basics#vcluster-syncer
 type Syncer struct {
 	KubeConfigContextName string                   `json:"kubeConfigContextName,omitempty"`
@@ -179,7 +179,7 @@ type MapServices struct {
 	FromVirtual []MapServicesFromVirtual `json:"fromVirtual"`
 }
 
-// VClusterIsolation - parameters to define the isolation of the cluster
+// Isolation - parameters to define the isolation of the cluster
 type Isolation struct {
 	Enabled             bool          `json:"enabled,omitempty"`
 	PodSecurityStandard string        `json:"podSecurityStandard,omitempty"`
@@ -188,7 +188,7 @@ type Isolation struct {
 	NetworkPolicy       NetworkPolicy `json:"networkPolicy,omitempty"`
 }
 
-// VClusterNodeSelector - parameters to define the node selector of the cluster
+// NodeSelector - parameters to define the node selector of the cluster
 type NodeSelector struct {
 	SandboxGKEIORuntime string `json:"sandbox.gke.io/runtime"`
 }
@@ -197,7 +197,7 @@ type SecurityContextCapabilities struct {
 	Drop []string `json:"drop"`
 }
 
-// VClusterSecurityContext - parameters to define the security context of the cluster
+// SecurityContext - parameters to define the security context of the cluster
 type SecurityContext struct {
 	Capabilities           SecurityContextCapabilities `json:"capabilities"`
 	ReadOnlyRootFilesystem bool                        `json:"readOnlyRootFilesystem"`

controllers/uffizzicluster/conditions_test.go

@@ -19,3 +19,83 @@ func TestBuildInitializingCondition(t *testing.T) {
 		t.Errorf("Expected UffizziCluster is being initialized, got %s", initCondition.Message)
 	}
 }
+
+func TestBuildInitializingAPICondition(t *testing.T) {
+	initCondition := InitializingAPI()
+	if initCondition.Type != "APIReady" {
+		t.Errorf("Expected APIReady, got %s", initCondition.Type)
+	}
+	if initCondition.Status != "Unknown" {
+		t.Errorf("Expected Unknown, got %s", initCondition.Status)
+	}
+	if initCondition.Reason != "Initializing" {
+		t.Errorf("Expected Initializing, got %s", initCondition.Reason)
+	}
+	if initCondition.Message != "UffizziCluster is being initialized" {
+		t.Errorf("Expected UffizziCluster is being initialized, got %s", initCondition.Message)
+	}
+}
+
+func TestBuildInitializingDataStoreCondition(t *testing.T) {
+	initCondition := InitializingDataStore()
+	if initCondition.Type != "DataStoreReady" {
+		t.Errorf("Expected DataStoreReady, got %s", initCondition.Type)
+	}
+	if initCondition.Status != "Unknown" {
+		t.Errorf("Expected Unknown, got %s", initCondition.Status)
+	}
+	if initCondition.Reason != "Initializing" {
+		t.Errorf("Expected Initializing, got %s", initCondition.Reason)
+	}
+	if initCondition.Message != "UffizziCluster is being initialized" {
+		t.Errorf("Expected UffizziCluster is being initialized, got %s", initCondition.Message)
+	}
+}
+
+func TestBuildAPIReadyCondition(t *testing.T) {
+	initCondition := APIReady()
+	if initCondition.Type != "APIReady" {
+		t.Errorf("Expected APIReady, got %s", initCondition.Type)
+	}
+	if initCondition.Status != "True" {
+		t.Errorf("Expected True, got %s", initCondition.Status)
+	}
+	if initCondition.Reason != "APIReady" {
+		t.Errorf("Expected APIReady, got %s", initCondition.Reason)
+	}
+	if initCondition.Message != "UffizziCluster API is ready" {
+		t.Errorf("Expected UffizziCluster API is ready, got %s", initCondition.Message)
+	}
+}
+
+func TestDataStoreReadyCondition(t *testing.T) {
+	initCondition := DataStoreReady()
+	if initCondition.Type != "DataStoreReady" {
+		t.Errorf("Expected DataStoreReady, got %s", initCondition.Type)
+	}
+	if initCondition.Status != "True" {
+		t.Errorf("Expected True, got %s", initCondition.Status)
+	}
+	if initCondition.Reason != "DataStoreReady" {
+		t.Errorf("Expected DataStoreReady, got %s", initCondition.Reason)
+	}
+	if initCondition.Message != "UffizziCluster Datastore is ready" {
+		t.Errorf("Expected UffizziCluster Datastore is ready, got %s", initCondition.Message)
+	}
+}
+
+func TestAPINotReadyCondition(t *testing.T) {
+	initCondition := APINotReady()
+	if initCondition.Type != "APIReady" {
+		t.Errorf("Expected APIReady, got %s", initCondition.Type)
+	}
+	if initCondition.Status != "False" {
+		t.Errorf("Expected False, got %s", initCondition.Status)
+	}
+	if initCondition.Reason != "APINotReady" {
+		t.Errorf("Expected APINotReady, got %s", initCondition.Reason)
+	}
+	if initCondition.Message != "UffizziCluster API is not ready" {
+		t.Errorf("Expected UffizziCluster API is not ready, got %s", initCondition.Message)
+	}
+}

controllers/uffizzicluster/deployment.go

@@ -2,6 +2,7 @@ package uffizzicluster
 
 import (
 	"context"
+	"errors"
 	v1alpha1 "github.com/UffizziCloud/uffizzi-cluster-operator/api/v1alpha1"
 	"github.com/UffizziCloud/uffizzi-cluster-operator/controllers/helm/build/vcluster"
 	appsv1 "k8s.io/api/apps/v1"
@@ -35,6 +36,9 @@ func (r *UffizziClusterReconciler) scaleDeployments(ctx context.Context, scale i
 
 // waitForStatefulSetToScale is a goroutine which waits for the stateful set to be ready
 func (r *UffizziClusterReconciler) waitForDeploymentToScale(ctx context.Context, scale int, ucDeployment *appsv1.Deployment) error {
+	if ucDeployment == nil {
+		return errors.New("deployment is nil")
+	}
 	// wait for the Deployment to be ready
 	return wait.PollImmediate(time.Second*5, time.Minute*1, func() (bool, error) {
 		if err := r.Get(ctx, types.NamespacedName{

controllers/uffizzicluster/statefulset.go

@@ -2,6 +2,7 @@ package uffizzicluster
 
 import (
 	"context"
+	"errors"
 	"github.com/UffizziCloud/uffizzi-cluster-operator/api/v1alpha1"
 	uffizzicluster "github.com/UffizziCloud/uffizzi-cluster-operator/controllers/etcd"
 	"github.com/UffizziCloud/uffizzi-cluster-operator/controllers/helm/build/vcluster"
@@ -36,6 +37,9 @@ func (r *UffizziClusterReconciler) scaleStatefulSets(ctx context.Context, scale
 	// if the current replicas is greater than 0, then scale down to 0
 	replicas := int32(scale)
 	for _, ss := range statefulSets {
+		if ss == nil {
+			return errors.New("statefulSet is nil")
+		}
 		ss.Spec.Replicas = &replicas
 		if err := r.Update(ctx, ss); err != nil {
 			return err
@@ -46,6 +50,9 @@ func (r *UffizziClusterReconciler) scaleStatefulSets(ctx context.Context, scale
 
 // waitForStatefulSetToScale is a goroutine which waits for the stateful set to be ready
 func (r *UffizziClusterReconciler) waitForStatefulSetToScale(ctx context.Context, scale int, ucStatefulSet *appsv1.StatefulSet) error {
+	if ucStatefulSet == nil {
+		return errors.New("statefulSet is nil")
+	}
 	// wait for the StatefulSet to be ready
 	return wait.PollImmediate(time.Second*5, time.Minute*1, func() (bool, error) {
 		if err := r.Get(ctx, types.NamespacedName{

controllers/uffizzicluster/uffizzicluster_controller.go

@@ -245,7 +245,7 @@ func (r *UffizziClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 				}
 			} else {
 				if updatedHelmRelease, err = r.upsertVClusterK3SHelmRelease(true, ctx, uCluster); err != nil {
-					logger.Error(err, "Failed to update HelmRelease")
+					logger.Info("Failed to update HelmRelease with error, reconciling", "Error", err.Error())
 					return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 5}, err
 				}
 			}
@@ -262,7 +262,7 @@ func (r *UffizziClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 			// logger.Info("vcluster statefulset not found, requeueing")
 			return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil
 		}
-		logger.Error(err, "Failed to reconcile sleep state")
+		logger.Info("Failed to reconcile sleep state, reconciling", "Error", err.Error())
 		return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil
 	}
 
@@ -324,13 +324,17 @@ func (r *UffizziClusterReconciler) reconcileSleepState(ctx context.Context, uClu
 		currentReplicas := ucStatefulSet.Spec.Replicas
 		// scale the vcluster instance to 0 if the sleep flag is true
 		if uCluster.Spec.Sleep && *currentReplicas > 0 {
-			if err := r.waitForStatefulSetToScale(ctx, 0, ucStatefulSet); err == nil {
+			var err error
+			if err = r.waitForStatefulSetToScale(ctx, 0, ucStatefulSet); err == nil {
 				setCondition(uCluster, APINotReady())
 			}
-			if err := r.waitForStatefulSetToScale(ctx, 0, etcdStatefulSet); err == nil {
+			if err = r.waitForStatefulSetToScale(ctx, 0, etcdStatefulSet); err == nil {
 				setCondition(uCluster, DataStoreNotReady())
 			}
-			err := r.deleteWorkloads(ctx, uCluster)
+			if err != nil {
+				return err
+			}
+			err = r.deleteWorkloads(ctx, uCluster)
 			if err != nil {
 				return err
 			}
@@ -349,25 +353,34 @@ func (r *UffizziClusterReconciler) reconcileSleepState(ctx context.Context, uClu
 			uCluster.Status.LastAwakeTime = lastAwakeTime
 			// if the above runs successfully, then set the status to awake
 			setCondition(uCluster, Awoken(lastAwakeTime))
-			if err := r.waitForStatefulSetToScale(ctx, 1, etcdStatefulSet); err == nil {
+			var err error
+			if err = r.waitForStatefulSetToScale(ctx, 1, etcdStatefulSet); err == nil {
 				setCondition(uCluster, APIReady())
 			}
-			if err := r.waitForStatefulSetToScale(ctx, 1, ucStatefulSet); err == nil {
+			if err = r.waitForStatefulSetToScale(ctx, 1, ucStatefulSet); err == nil {
 				setCondition(uCluster, DataStoreReady())
 			}
+			if err != nil {
+				return err
+			}
 		}
 	case *appsv1.Deployment:
 		ucDeployment := ucWorkload.(*appsv1.Deployment)
 		currentReplicas := ucDeployment.Spec.Replicas
 		// scale the vcluster instance to 0 if the sleep flag is true
 		if uCluster.Spec.Sleep && *currentReplicas > 0 {
-			if err := r.waitForDeploymentToScale(ctx, 0, ucDeployment); err == nil {
+			var err error
+			if err = r.waitForDeploymentToScale(ctx, 0, ucDeployment); err == nil {
 				setCondition(uCluster, APINotReady())
 			}
-			if err := r.waitForStatefulSetToScale(ctx, 0, etcdStatefulSet); err == nil {
+			if err = r.waitForStatefulSetToScale(ctx, 0, etcdStatefulSet); err == nil {
 				setCondition(uCluster, DataStoreNotReady())
 			}
-			err := r.deleteWorkloads(ctx, uCluster)
+			if err != nil {
+				return err
+			}
+
+			err = r.deleteWorkloads(ctx, uCluster)
 			if err != nil {
 				return err
 			}
@@ -386,16 +399,20 @@ func (r *UffizziClusterReconciler) reconcileSleepState(ctx context.Context, uClu
 			uCluster.Status.LastAwakeTime = lastAwakeTime
 			// if the above runs successfully, then set the status to awake
 			setCondition(uCluster, Awoken(lastAwakeTime))
-			if err := r.waitForStatefulSetToScale(ctx, 1, etcdStatefulSet); err == nil {
+			var err error
+			if err = r.waitForStatefulSetToScale(ctx, 1, etcdStatefulSet); err == nil {
 				setCondition(uCluster, APIReady())
 			}
-			if err := r.waitForDeploymentToScale(ctx, 1, ucDeployment); err == nil {
+			if err = r.waitForDeploymentToScale(ctx, 1, ucDeployment); err == nil {
 				setCondition(uCluster, DataStoreReady())
 			}
+			if err != nil {
+				return err
+			}
 		}
 
 	default:
-		return errors.New("unknown workload type  for vcluster")
+		return errors.New("unknown workload type for vcluster")
 	}
 	if err := r.Status().Patch(ctx, uCluster, patch); err != nil {
 		return err