Enable replicas override #1178

Richard87 · 2024-08-23T08:32:26Z

Probably needs to be released together with Radix-API

…onent-type

satr · 2024-09-10T07:39:14Z

pkg/apis/kube/kube.go

@@ -53,7 +54,8 @@ const (
 	RadixCommitLabel                    = "radix-commit"
 	RadixImageTagLabel                  = "radix-image-tag"
 	RadixJobTypeLabel                   = "radix-job-type"
-	RadixJobTypeJob                     = "job" // Outer job
+	RadixJobTypeJob                     = "job"       // Outer job
+	RadixJobTypeAuxJobSleep             = "job-sleep" // Outer job


What is this label for and why is it called "Sleep"?

Renamed to job-manager-auxiliary

satr · 2024-09-10T08:11:34Z

pkg/apis/deployment/kubedeployment.go

+	if override := deployComponent.GetReplicasOverride(); override != nil {
+		componentReplicas = int32(*override)
+	}
+


Wouldn't be this method actually this way (and the name of the method is probably not longer consistent, as it participates in garbage-collect for PodDisruption):

func getDesiredComponentReplicas(deployComponent v1.RadixCommonDeployComponent) *int32 { if override := deployComponent.GetReplicasOverride(); override != nil { return pointers.Ptr(int32(*override)) } componentReplicas := int32(DefaultReplicas) if replicas := deployComponent.GetReplicas(); replicas != nil { componentReplicas = int32(*replicas) } if hs := deployComponent.GetHorizontalScaling(); hs != nil { if hs.MinReplicas != nil && *hs.MinReplicas > componentReplicas { componentReplicas = *hs.MinReplicas } if hs.MaxReplicas < componentReplicas { componentReplicas = hs.MaxReplicas } } return pointers.Ptr(componentReplicas) }

fixed:

func getDeployComponentReplicas(deployComponent v1.RadixCommonDeployComponent) int32 { if override := deployComponent.GetReplicasOverride(); override != nil { return int32(*override) } componentReplicas := DefaultReplicas if replicas := deployComponent.GetReplicas(); replicas != nil { componentReplicas = int32(*replicas) } if hs := deployComponent.GetHorizontalScaling(); hs != nil { if hs.MinReplicas != nil && *hs.MinReplicas > componentReplicas { return *hs.MinReplicas } if hs.MaxReplicas < componentReplicas { return hs.MaxReplicas } } return componentReplicas }

satr · 2024-09-10T08:24:04Z

pkg/apis/utils/annotations/annotations.go

@@ -27,6 +28,13 @@ func ForRadixBranch(branch string) map[string]string {
 func ForRadixDeploymentName(deploymentName string) map[string]string {
 	return map[string]string{kube.RadixDeploymentNameAnnotation: deploymentName}
 }
+func ForKubernetesDeploymentObservedGeneration(rd *radixv1.RadixDeployment) map[string]string {


Missing comment.
What is this annotation for?

satr · 2024-09-10T08:31:23Z

pkg/apis/deployment/pdb.go

-			*horizontalScaling.MinReplicas > 1
-	}
-	return replicas > 1
+	return *getDesiredComponentReplicas(component) > 1


This code is potentially failful - check the nil-able value on nil

satr · 2024-09-10T08:34:22Z

pkg/apis/deployment/scaledobject.go

+	if deployComponent.GetReplicasOverride() != nil {
+		log.Ctx(ctx).Debug().Msgf("Skip creating ScaledObject %s in namespace %s: manuall override is set", componentName, namespace)
+		return nil
+	}


Move this to the top of the method (optional, but more logical for reading)

satr · 2024-09-10T08:36:06Z

pkg/apis/kube/kube.go

@@ -28,6 +28,7 @@ const (
 	RadixDeploymentNameAnnotation                    = "radix-deployment-name"
 	RadixDeploymentPromotedFromDeploymentAnnotation  = "radix.equinor.com/radix-deployment-promoted-from-deployment"
 	RadixDeploymentPromotedFromEnvironmentAnnotation = "radix.equinor.com/radix-deployment-promoted-from-environment"
+	RadixDeploymentObservedGeneration                = "radix.equinor.com/radix-deployment-observed-generation"


Suggestion to add a description comment

satr · 2024-09-10T08:37:12Z

pkg/apis/radix/v1/radixdeploytypes.go

+	Image            string          `json:"image"`
+	Ports            []ComponentPort `json:"ports"`
+	Replicas         *int            `json:"replicas"`
+	ReplicasOverride *int            `json:"replicasOverride"`


Suggested change

ReplicasOverride *int `json:"replicasOverride"`

ReplicasOverride *int `json:"replicasOverride,omitempty"`

satr · 2024-09-10T08:39:28Z

pkg/apis/radixvalidators/validate_rd.go

@@ -76,6 +76,10 @@ func validateDeployComponents(deployment *radixv1.RadixDeployment) []error {
 			errs = append(errs, err)
 		}

+		if err := validateReplica(component.ReplicasOverride); err != nil {


It can be confusing to show the error for the replicas property. Maybe better to have a separate error

satr · 2024-09-10T14:08:12Z

pkg/apis/deployment/kubedeployment.go

 	if replicas := deployComponent.GetReplicas(); replicas != nil {
 		componentReplicas = int32(*replicas)
 	}

 	if hs := deployComponent.GetHorizontalScaling(); hs != nil {
 		if hs.MinReplicas != nil && *hs.MinReplicas > componentReplicas {
-			componentReplicas = *hs.MinReplicas
+			return *hs.MinReplicas


This should not be changed - next if used its value
componentReplicas = *hs.MinReplicas

Are you sure?

The block

if hs.MinReplicas != nil && *hs.MinReplicas > componentReplicas { componentReplicas = *hs.MinReplicas }

Would basically return minReplicas if its above componentReplicas?

satr · 2024-09-10T14:09:07Z

pkg/apis/deployment/kubedeployment.go

 		}
 		if hs.MaxReplicas < componentReplicas {
-			componentReplicas = hs.MaxReplicas
+			return hs.MaxReplicas


So maybe this would be also good to have similar componentReplicas = hs.MaxReplicas - for easy reading

same as the other one:

if hs.MaxReplicas < componentReplicas { componentReplicas = hs.MaxReplicas }

Would basically return Max if its less than componentReplicas 🤔

Richard87 added 3 commits August 23, 2024 10:30

Enable ReplicasOverride flag to manually change replicas

d077f1a

add replicasOverride if active deployment have overrides

d9a61e4

delete, and skip creating ScaledObjects if overrides is configured

e2ad61b

Richard87 self-assigned this Aug 23, 2024

Richard87 added 9 commits August 23, 2024 12:14

validate max / min replica override

56df99f

Merge branch 'master' into enable-replicas-override

f9348df

Add replicas override test for active deployment

1a376be

typo

e94c0a6

Add observed annotation to k8s deployment

b25d571

add test for observed generation

10802ff

bugfix swagger types

09aee0e

Add observed RD Version in deployments

52188ea

replace component with aux-component and component-type with aux-comp…

a77147f

…onent-type

Richard87 requested a review from satr September 3, 2024 09:51

Richard87 added 3 commits September 3, 2024 11:51

Merge branch 'master' into enable-replicas-override

cf7d4ef

replace component with aux-component and component-type with aux-comp…

1624831

…onent-type

Bump chart version

a20026a

Richard87 marked this pull request as ready for review September 3, 2024 09:55

Richard87 added 2 commits September 3, 2024 12:03

handle error

504a6b7

upgrade golangcilint

bce7143

satr reviewed Sep 10, 2024

View reviewed changes

emirgens mentioned this pull request Sep 10, 2024

Ensure the scaling configuration has right config #1158

Open

fixed comments

a44c3b3

cleanup getDeployComponentReplicas

c097358

satr reviewed Sep 10, 2024

View reviewed changes

fix test

0beedce

satr approved these changes Sep 11, 2024

View reviewed changes

Richard87 merged commit a7a1e27 into master Sep 11, 2024
7 checks passed

Richard87 deleted the enable-replicas-override branch September 11, 2024 05:49

Richard87 mentioned this pull request Sep 12, 2024

Manuall Scaling of Replicas equinor/radix-public-site#387

Closed

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable replicas override #1178

Enable replicas override #1178

Richard87 commented Aug 23, 2024 •

edited

Loading

satr Sep 10, 2024

Richard87 Sep 10, 2024

Richard87 Sep 10, 2024

satr Sep 10, 2024 •

edited

Loading

Richard87 Sep 10, 2024

satr Sep 10, 2024 •

edited

Loading

Richard87 Sep 10, 2024

satr Sep 10, 2024

Richard87 Sep 10, 2024

satr Sep 10, 2024

Richard87 Sep 10, 2024

satr Sep 10, 2024

Richard87 Sep 10, 2024

satr Sep 10, 2024

Richard87 Sep 10, 2024

satr Sep 10, 2024

Richard87 Sep 10, 2024

satr Sep 10, 2024

Richard87 Sep 11, 2024

satr Sep 10, 2024

Richard87 Sep 11, 2024

	ReplicasOverride *int `json:"replicasOverride"`
	ReplicasOverride *int `json:"replicasOverride,omitempty"`

Enable replicas override #1178

Enable replicas override #1178

Conversation

Richard87 commented Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satr Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satr Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Richard87 commented Aug 23, 2024 •

edited

Loading

satr Sep 10, 2024 •

edited

Loading

satr Sep 10, 2024 •

edited

Loading