chore: review

Signed-off-by: Leonardo Cecchi <[email protected]>

Author: leonardoce

.wordlist.txt

@@ -26,6 +26,7 @@ ObjectStoreStatus
 PITR
 README
 RecoveryWindow
+ResourceRequirements
 RetentionPolicy
 SAS
 SFO
@@ -54,6 +55,7 @@ cmctl
 cnpg
 codebase
 containerPort
+cpu
 creds
 csi
 customresourcedefinition

api/v1/objectstore_types.go

@@ -33,9 +33,10 @@ type InstanceSidecarConfiguration struct {
 	// +kubebuilder:default:=1800
 	// +optional
 	RetentionPolicyIntervalSeconds int `json:"retentionPolicyIntervalSeconds,omitempty"`
+
 	// Resources define cpu/memory requests and limits for the sidecar that runs in the instance pods.
-        // +optional
-        Resources corev1.ResourceRequirements `json:"resources,omitempty"`
+	// +optional
+	Resources corev1.ResourceRequirements `json:"resources,omitempty"`
 }
 
 // ObjectStoreSpec defines the desired state of ObjectStore.

api/v1/zz_generated.deepcopy.go

@@ -506,18 +506,41 @@ spec:
                       - name
                       type: object
                     type: array
-                  retentionPolicyIntervalSeconds:
-                    default: 1800
-                    description: |-
-                      The retentionCheckInterval defines the frequency at which the
-                      system checks and enforces retention policies.
-                    type: integer
                   resources:
-                    description: |-
-                      Plugin's sidecar resources requirements. Please refer to
-                      https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
-                      for more information.
+                    description: Resources define cpu/memory requests and limits for
+                      the sidecar that runs in the instance pods.
                     properties:
+                      claims:
+                        description: |-
+                          Claims lists the names of resources, defined in spec.resourceClaims,
+                          that are used by this container.
+
+                          This is an alpha field and requires enabling the
+                          DynamicResourceAllocation feature gate.
+
+                          This field is immutable. It can only be set for containers.
+                        items:
+                          description: ResourceClaim references one entry in PodSpec.ResourceClaims.
+                          properties:
+                            name:
+                              description: |-
+                                Name must match the name of one entry in pod.spec.resourceClaims of
+                                the Pod where this field is used. It makes that resource available
+                                inside a container.
+                              type: string
+                            request:
+                              description: |-
+                                Request is the name chosen for a request in the referenced claim.
+                                If empty, everything from the claim is made available, otherwise
+                                only the result of this request.
+                              type: string
+                          required:
+                          - name
+                          type: object
+                        type: array
+                        x-kubernetes-list-map-keys:
+                        - name
+                        x-kubernetes-list-type: map
                       limits:
                         additionalProperties:
                           anyOf:
@@ -543,6 +566,12 @@ spec:
                           More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
                         type: object
                     type: object
+                  retentionPolicyIntervalSeconds:
+                    default: 1800
+                    description: |-
+                      The retentionCheckInterval defines the frequency at which the
+                      system checks and enforces retention policies.
+                    type: integer
                 type: object
               retentionPolicy:
                 description: |-

config/crd/bases/barmancloud.cnpg.io_objectstores.yaml

@@ -6,6 +6,13 @@ spec:
   retentionPolicy: "1m"
   instanceSidecarConfiguration:
     retentionPolicyIntervalSeconds: 30
+    resources:
+      requests:
+        memory: "64Mi"
+        cpu: "250m"
+      limits:
+        memory: "128Mi"
+        cpu: "500m"
   configuration:
     endpointCA:
       name: minio-server-tls

hack/examples/minio-store.yaml

@@ -17,6 +17,7 @@ import (
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
 	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
 	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
 )
@@ -123,23 +124,36 @@ func (impl LifecycleImplementation) reconcileJob(
 		return nil, err
 	}
 
-	return reconcileJob(ctx, cluster, request, env, certificates)
+	resources, err := impl.collectSidecarResources(ctx, pluginConfiguration)
+	if err != nil {
+		return nil, err
+	}
+
+	return reconcileJob(ctx, cluster, request, sidecarConfiguration{
+		env:          env,
+		certificates: certificates,
+		resources:    resources,
+	})
+}
+
+type sidecarConfiguration struct {
+	env          []corev1.EnvVar
+	certificates []corev1.VolumeProjection
+	resources    corev1.ResourceRequirements
 }
 
 func reconcileJob(
 	ctx context.Context,
 	cluster *cnpgv1.Cluster,
 	request *lifecycle.OperatorLifecycleRequest,
-	env []corev1.EnvVar,
-	certificates []corev1.VolumeProjection,
+	config sidecarConfiguration,
 ) (*lifecycle.OperatorLifecycleResponse, error) {
 	contextLogger := log.FromContext(ctx).WithName("lifecycle")
 	if pluginConfig := cluster.GetRecoverySourcePlugin(); pluginConfig == nil || pluginConfig.Name != metadata.PluginName {
 		contextLogger.Debug("cluster does not use the this plugin for recovery, skipping")
 		return nil, nil
 	}
 
-	var barmanObjectStore barmancloudv1.ObjectStore
 	var job batchv1.Job
 	if err := decoder.DecodeObjectStrict(
 		request.GetObjectDefinition(),
@@ -170,9 +184,7 @@ func reconcileJob(
 		corev1.Container{
 			Args: []string{"restore"},
 		},
-		env,
-		certificates,
-		barmanObjectStore,
+		config,
 	); err != nil {
 		return nil, fmt.Errorf("while reconciling pod spec for job: %w", err)
 	}
@@ -204,23 +216,36 @@ func (impl LifecycleImplementation) reconcilePod(
 		return nil, err
 	}
 
+	resources, err := impl.collectSidecarResources(ctx, pluginConfiguration)
+	if err != nil {
+		return nil, err
+	}
+
+	return reconcilePod(ctx, cluster, request, pluginConfiguration, sidecarConfiguration{
+		env:          env,
+		certificates: certificates,
+		resources:    resources,
+	})
+}
+
+func (impl LifecycleImplementation) collectSidecarResources(
+	ctx context.Context,
+	configuration *config.PluginConfiguration,
+) (corev1.ResourceRequirements, error) {
 	var barmanObjectStore barmancloudv1.ObjectStore
-  	configuration := config.NewFromCluster(cluster)
-  	if err := impl.Client.Get(ctx, configuration.GetBarmanObjectKey(), &barmanObjectStore); err != nil {
-    	return nil, err
-  	}
+	if err := impl.Client.Get(ctx, configuration.GetBarmanObjectKey(), &barmanObjectStore); err != nil {
+		return corev1.ResourceRequirements{}, err
+	}
 
-	return reconcilePod(ctx, cluster, request, pluginConfiguration, env, certificates, barmanObjectStore)
+	return barmanObjectStore.Spec.InstanceSidecarConfiguration.Resources, nil
 }
 
 func reconcilePod(
 	ctx context.Context,
 	cluster *cnpgv1.Cluster,
 	request *lifecycle.OperatorLifecycleRequest,
 	pluginConfiguration *config.PluginConfiguration,
-	env []corev1.EnvVar,
-	certificates []corev1.VolumeProjection,
-	barmanObjectStore barmancloudv1.ObjectStore,
+	config sidecarConfiguration,
 ) (*lifecycle.OperatorLifecycleResponse, error) {
 	pod, err := decoder.DecodePodJSON(request.GetObjectDefinition())
 	if err != nil {
@@ -240,9 +265,7 @@ func reconcilePod(
 			corev1.Container{
 				Args: []string{"instance"},
 			},
-			env,
-			certificates,
-			barmanObjectStore,
+			config,
 		); err != nil {
 			return nil, fmt.Errorf("while reconciling pod spec for pod: %w", err)
 		}
@@ -265,10 +288,8 @@ func reconcilePodSpec(
 	cluster *cnpgv1.Cluster,
 	spec *corev1.PodSpec,
 	mainContainerName string,
-	sidecarConfig corev1.Container,
-	additionalEnvs []corev1.EnvVar,
-	certificates []corev1.VolumeProjection,
-	barmanObjectStore barmancloudv1.ObjectStore,
+	sidecarTemplate corev1.Container,
+	config sidecarConfiguration,
 ) error {
 	envs := []corev1.EnvVar{
 		{
@@ -295,7 +316,7 @@ func reconcilePodSpec(
 		},
 	}
 
-	envs = append(envs, additionalEnvs...)
+	envs = append(envs, config.env...)
 
 	baseProbe := &corev1.Probe{
 		FailureThreshold: 10,
@@ -308,11 +329,11 @@ func reconcilePodSpec(
 	}
 
 	// fixed values
-	sidecarConfig.Name = "plugin-barman-cloud"
-	sidecarConfig.Image = viper.GetString("sidecar-image")
-	sidecarConfig.ImagePullPolicy = cluster.Spec.ImagePullPolicy
-	sidecarConfig.StartupProbe = baseProbe.DeepCopy()
-	sidecarConfig.SecurityContext = &corev1.SecurityContext{
+	sidecarTemplate.Name = "plugin-barman-cloud"
+	sidecarTemplate.Image = viper.GetString("sidecar-image")
+	sidecarTemplate.ImagePullPolicy = cluster.Spec.ImagePullPolicy
+	sidecarTemplate.StartupProbe = baseProbe.DeepCopy()
+	sidecarTemplate.SecurityContext = &corev1.SecurityContext{
 		AllowPrivilegeEscalation: ptr.To(false),
 		RunAsNonRoot:             ptr.To(true),
 		Privileged:               ptr.To(false),
@@ -324,21 +345,21 @@ func reconcilePodSpec(
 			Drop: []corev1.Capability{"ALL"},
 		},
 	}
-	sidecarConfig.Resources = barmanObjectStore.Spec.InstanceSidecarConfiguration.Resources
+	sidecarTemplate.Resources = config.resources
 
 	// merge the main container envs if they aren't already set
 	for _, container := range spec.Containers {
 		if container.Name == mainContainerName {
 			for _, env := range container.Env {
 				found := false
-				for _, existingEnv := range sidecarConfig.Env {
+				for _, existingEnv := range sidecarTemplate.Env {
 					if existingEnv.Name == env.Name {
 						found = true
 						break
 					}
 				}
 				if !found {
-					sidecarConfig.Env = append(sidecarConfig.Env, env)
+					sidecarTemplate.Env = append(sidecarTemplate.Env, env)
 				}
 			}
 			break
@@ -348,18 +369,18 @@ func reconcilePodSpec(
 	// merge the default envs if they aren't already set
 	for _, env := range envs {
 		found := false
-		for _, existingEnv := range sidecarConfig.Env {
+		for _, existingEnv := range sidecarTemplate.Env {
 			if existingEnv.Name == env.Name {
 				found = true
 				break
 			}
 		}
 		if !found {
-			sidecarConfig.Env = append(sidecarConfig.Env, env)
+			sidecarTemplate.Env = append(sidecarTemplate.Env, env)
 		}
 	}
 
-	if err := injectPluginSidecarPodSpec(spec, &sidecarConfig, mainContainerName); err != nil {
+	if err := injectPluginSidecarPodSpec(spec, &sidecarTemplate, mainContainerName); err != nil {
 		return err
 	}
 
@@ -369,7 +390,7 @@ func reconcilePodSpec(
 			Name: barmanCertificatesVolumeName,
 			VolumeSource: corev1.VolumeSource{
 				Projected: &corev1.ProjectedVolumeSource{
-					Sources: certificates,
+					Sources: config.certificates,
 				},
 			},
 		})

internal/cnpgi/operator/lifecycle.go

@@ -107,7 +107,7 @@ var _ = Describe("LifecycleImplementation", func() {
 				ObjectDefinition: jobJSON,
 			}
 
-			response, err := reconcileJob(ctx, cluster, request, nil, nil)
+			response, err := reconcileJob(ctx, cluster, request, sidecarConfiguration{})
 			Expect(err).NotTo(HaveOccurred())
 			Expect(response).NotTo(BeNil())
 			Expect(response.JsonPatch).NotTo(BeEmpty())
@@ -128,7 +128,7 @@ var _ = Describe("LifecycleImplementation", func() {
 				ObjectDefinition: jobJSON,
 			}
 
-			response, err := reconcileJob(ctx, cluster, request, nil, nil)
+			response, err := reconcileJob(ctx, cluster, request, sidecarConfiguration{})
 			Expect(err).NotTo(HaveOccurred())
 			Expect(response).To(BeNil())
 		})
@@ -138,7 +138,7 @@ var _ = Describe("LifecycleImplementation", func() {
 				ObjectDefinition: []byte("invalid-json"),
 			}
 
-			response, err := reconcileJob(ctx, cluster, request, nil, nil)
+			response, err := reconcileJob(ctx, cluster, request, sidecarConfiguration{})
 			Expect(err).To(HaveOccurred())
 			Expect(response).To(BeNil())
 		})
@@ -165,7 +165,7 @@ var _ = Describe("LifecycleImplementation", func() {
 					ObjectDefinition: jobJSON,
 				}
 
-				response, err := reconcileJob(ctx, cluster, request, nil, nil)
+				response, err := reconcileJob(ctx, cluster, request, sidecarConfiguration{})
 				Expect(err).NotTo(HaveOccurred())
 				Expect(response).To(BeNil())
 			})
@@ -185,7 +185,7 @@ var _ = Describe("LifecycleImplementation", func() {
 				ObjectDefinition: podJSON,
 			}
 
-			response, err := reconcilePod(ctx, cluster, request, pluginConfiguration, nil, nil)
+			response, err := reconcilePod(ctx, cluster, request, pluginConfiguration, sidecarConfiguration{})
 			Expect(err).NotTo(HaveOccurred())
 			Expect(response).NotTo(BeNil())
 			Expect(response.JsonPatch).NotTo(BeEmpty())
@@ -203,7 +203,7 @@ var _ = Describe("LifecycleImplementation", func() {
 				ObjectDefinition: []byte("invalid-json"),
 			}
 
-			response, err := reconcilePod(ctx, cluster, request, pluginConfiguration, nil, nil)
+			response, err := reconcilePod(ctx, cluster, request, pluginConfiguration, sidecarConfiguration{})
 			Expect(err).To(HaveOccurred())
 			Expect(response).To(BeNil())
 		})