Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1alpha1/argorollouts_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type RolloutManagerSpec struct {

// Metadata to apply to the generated resources
AdditionalMetadata *ResourceMetadata `json:"additionalMetadata,omitempty"`

// Resources requests/limits for Argo Rollout controller
ControllerResources *corev1.ResourceRequirements `json:"controllerResources,omitempty"`
}

// ArgoRolloutsNodePlacementSpec is used to specify NodeSelector and Tolerations for Rollouts workloads
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 55 additions & 0 deletions config/crd/bases/argoproj.io_rolloutmanagers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,61 @@ spec:
description: Labels to add to the resources during its creation.
type: object
type: object
controllerResources:
description: Resources requests/limits for Argo Rollout controller
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
required:
- name
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
limits:
additionalProperties:
anyOf:
- type: integer
- type: string
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
description: |-
Limits describes the maximum amount of compute resources allowed.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
type: object
requests:
additionalProperties:
anyOf:
- type: integer
- type: string
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
description: |-
Requests describes the minimum amount of compute resources required.
If Requests is omitted for a container, it defaults to Limits if that is explicitly specified,
otherwise to an implementation-defined value. Requests cannot exceed Limits.
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
type: object
type: object
env:
description: Env lets you specify environment for Rollouts pods
items:
Expand Down
21 changes: 16 additions & 5 deletions controllers/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,15 @@ func identifyDeploymentDifference(x appsv1.Deployment, y appsv1.Deployment) stri
return ""
}

// defaultRolloutsContainerResources return the default resource constaints set on containers, when the RolloutManager CR does not have resource constraints set.
func defaultRolloutsContainerResources() corev1.ResourceRequirements {
return corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
}
}

func rolloutsContainer(cr rolloutsmanagerv1alpha1.RolloutManager) corev1.Container {

// NOTE: When updating this function, ensure that normalizeDeployment is updated as well. See that function for details.
Expand All @@ -249,6 +258,12 @@ func rolloutsContainer(cr rolloutsmanagerv1alpha1.RolloutManager) corev1.Contain
// Environment specified in the CR take precedence over everything else
rolloutsEnv = envMerge(rolloutsEnv, proxyEnvVars(), false)

containerResources := cr.Spec.ControllerResources
if containerResources == nil {
defaultContainerResources := defaultRolloutsContainerResources()
containerResources = &defaultContainerResources
}

return corev1.Container{
Args: getRolloutsCommandArgs(cr),
Env: rolloutsEnv,
Expand Down Expand Up @@ -291,11 +306,6 @@ func rolloutsContainer(cr rolloutsmanagerv1alpha1.RolloutManager) corev1.Contain
SuccessThreshold: int32(1),
TimeoutSeconds: int32(4),
},
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
Expand All @@ -319,6 +329,7 @@ func rolloutsContainer(cr rolloutsmanagerv1alpha1.RolloutManager) corev1.Contain
Name: "tmp",
},
},
Resources: *containerResources,
}

}
Expand Down
99 changes: 99 additions & 0 deletions controllers/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ var _ = Describe("Deployment Test", func() {
Expect(fetchedDeployment.Spec.Template.Spec.Tolerations).To(Equal(expectedDeployment.Spec.Template.Spec.Tolerations))
Expect(fetchedDeployment.Spec.Template.Spec.SecurityContext).To(Equal(expectedDeployment.Spec.Template.Spec.SecurityContext))
Expect(fetchedDeployment.Spec.Template.Spec.Volumes).To(Equal(expectedDeployment.Spec.Template.Spec.Volumes))
Expect(fetchedDeployment.Spec.Template.Spec.Containers[0].Resources).To(Equal(expectedDeployment.Spec.Template.Spec.Containers[0].Resources))
})

When("Rollouts Deployment already exists, but then is modified away from default values", func() {
Expand Down Expand Up @@ -96,10 +97,96 @@ var _ = Describe("Deployment Test", func() {
Expect(fetchedDeployment.Spec.Template.Spec.Tolerations).To(Equal(expectedDeployment.Spec.Template.Spec.Tolerations))
Expect(fetchedDeployment.Spec.Template.Spec.SecurityContext).To(Equal(expectedDeployment.Spec.Template.Spec.SecurityContext))
Expect(fetchedDeployment.Spec.Template.Spec.Volumes).To(Equal(expectedDeployment.Spec.Template.Spec.Volumes))
Expect(fetchedDeployment.Spec.Template.Spec.Containers[0].Resources).To(Equal(expectedDeployment.Spec.Template.Spec.Containers[0].Resources))

})
})

When("RolloutManagerCR has custom controller resources defined", func() {

It("should create a Deployment that uses those controller resources", func() {

By("setting resource requirements on RolloutsManager CR")
a.Spec.ControllerResources = &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("500Mi"),
},
}
Expect(r.Client.Update(ctx, &a)).To(Succeed())

By("calling reconcileRolloutsDeployment to create the initial set of rollout resources")
Expect(r.reconcileRolloutsDeployment(ctx, a, *sa)).To(Succeed())

By("fetching the Deployment")
fetchedDeployment := &appsv1.Deployment{}
Expect(fetchObject(ctx, r.Client, a.Namespace, DefaultArgoRolloutsResourceName, fetchedDeployment)).To(Succeed())

By("verifying that the fetched Deployment matches the desired one")
Expect(fetchedDeployment.Spec.Template.Spec.Containers[0].Resources).To(Equal(*a.Spec.ControllerResources))
})

defaultContainerResources := defaultRolloutsContainerResources()

nonDefaultContainerResourcesValue := &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("500Mi"),
},
}

otherNonDefault := corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("1m"),
corev1.ResourceMemory: resource.MustParse("1Mi"),
},
}

DescribeTable("Deployment CR should always be updated to be consistent with RolloutsManager .spec.controllerResources field, in both default and non-default cases", func(initialDeployment *corev1.ResourceRequirements, crValue *corev1.ResourceRequirements, expectedDeployment *corev1.ResourceRequirements) {

By("calling reconcileRolloutsDeployment to create the initial set of rollout resources")
Expect(r.reconcileRolloutsDeployment(ctx, a, *sa)).To(Succeed())

By("updating the default Deployment to resource value defined in 'initialDeployment'")
fetchedDeployment := &appsv1.Deployment{}
Expect(fetchObject(ctx, r.Client, a.Namespace, DefaultArgoRolloutsResourceName, fetchedDeployment)).To(Succeed())

fetchedDeployment.Spec.Template.Spec.Containers[0].Resources = *initialDeployment
Expect(r.Update(ctx, fetchedDeployment)).To(Succeed())

if crValue != nil {
By("setting resource requirements on RolloutsManager CR to 'crValue'")
a.Spec.ControllerResources = crValue
Expect(r.Client.Update(ctx, &a)).To(Succeed())
}

By("calling reconcileRolloutsDeployment")
Expect(r.reconcileRolloutsDeployment(ctx, a, *sa)).To(Succeed())

By("fetching the Deployment")
fetchedDeployment = &appsv1.Deployment{}
Expect(fetchObject(ctx, r.Client, a.Namespace, DefaultArgoRolloutsResourceName, fetchedDeployment)).To(Succeed())

By("verifying that the fetched Deployment resource requirements matches the desired resource requirements")
Expect(fetchedDeployment.Spec.Template.Spec.Containers[0].Resources).To(Equal(*expectedDeployment))

},
Entry("default deployment, with a empty CR .spec.containerResources -> no change in deployment from default", &defaultContainerResources, nil, &defaultContainerResources),
Entry("default deployment, with CR non-default value in .spec.containerResources -> deployment should now have value from CR", &defaultContainerResources, nonDefaultContainerResourcesValue, nonDefaultContainerResourcesValue),
Entry("deployment with non-default container resources, empty value in CR .spec.containerResources -> Deployment should revert to default value from CR", nonDefaultContainerResourcesValue, nil, &defaultContainerResources),
Entry("deployment with a different non-default container resources, non-default value in CR .spec.containerResources -> Deployment should use CR value", &otherNonDefault, nonDefaultContainerResourcesValue, nonDefaultContainerResourcesValue),
)

})

When("Rollouts deployment already exists, but then RolloutManager is modified in a way that requires updating either .spec.selector of the existing Deployment", func() {

It("should cause the existing Deployment to be deleted, and a new Deployment to be created with the updated .spec.selector", func() {
Expand Down Expand Up @@ -315,6 +402,18 @@ var _ = Describe("Deployment Test", func() {
Entry(".spec.template.spec.volumes", func(deployment *appsv1.Deployment) {
deployment.Spec.Template.Spec.Volumes = []corev1.Volume{{Name: "my-volume"}}
}),
Entry(".spec.template.spec.containers.resources", func(deployment *appsv1.Deployment) {
deployment.Spec.Template.Spec.Containers[0].Resources = corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("500Mi"),
},
}
}),
)

})
Expand Down
21 changes: 21 additions & 0 deletions docs/crd_reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,24 @@ spec:
myannotation: "myvalue"
```


### RolloutManager example with resources requests/limits for the Argo Rollouts controller

You can provide resources requests and limits for the Argo Rollouts controller.

``` yaml
apiVersion: argoproj.io/v1alpha1
kind: RolloutManager
metadata:
name: argo-rollout
labels:
example: with-resources-example
spec:
controllerResources:
requests:
memory: "64Mi"
cpu: "250m"
limits:
memory: "128Mi"
cpu: "500m"
```
15 changes: 15 additions & 0 deletions examples/custom_resources_rolloutmanager.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
apiVersion: argoproj.io/v1alpha1
kind: RolloutManager
metadata:
name: argo-rollout
labels:
example: with-resources-example
spec:
controllerResources:
requests:
memory: "64Mi"
cpu: "250m"
limits:
memory: "128Mi"
cpu: "500m"
63 changes: 63 additions & 0 deletions tests/e2e/rollout_tests_all.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -443,5 +444,67 @@ func RunRolloutsTests(namespaceScopedParam bool) {
expectMetadataOnObjectMeta(&service.ObjectMeta, rolloutsManager.Spec.AdditionalMetadata)
})
})

When("A RolloutManager specifies controller resources under .spec.controllerResources", func() {

It("should create the controller with the correct resources requests/limits", func() {

By("creating a RolloutManager containing resource requirements")

rmWithResources := rolloutsmanagerv1alpha1.RolloutManager{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-rollouts-manager-with-resources",
Namespace: fixture.TestE2ENamespace,
},
Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{
ControllerResources: &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("500Mi"),
},
},
NamespaceScoped: namespaceScopedParam,
},
}

Expect(k8sClient.Create(ctx, &rmWithResources)).To(Succeed())

Eventually(rmWithResources, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))

deployment := appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultArgoRolloutsResourceName, Namespace: rmWithResources.Namespace},
}
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&deployment), &deployment)).To(Succeed())

Expect(deployment.Spec.Template.Spec.Containers[0].Resources).To(Equal(*rmWithResources.Spec.ControllerResources))

By("updating RolloutManager to use a different CPU limit")

err := k8s.UpdateWithoutConflict(ctx, &rmWithResources, k8sClient, func(obj client.Object) {
rm, ok := obj.(*rolloutsmanagerv1alpha1.RolloutManager)
Expect(ok).To(BeTrue())

rm.Spec.ControllerResources.Limits[corev1.ResourceCPU] = resource.MustParse("555m")

})
Expect(err).ToNot(HaveOccurred())

Eventually(func() bool {
deployment := appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultArgoRolloutsResourceName, Namespace: rmWithResources.Namespace},
}
if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(&deployment), &deployment); err != nil {
return false
}
return deployment.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] == resource.MustParse("555m")

}, "1m", "1s").Should(BeTrue(), "Deployment should switch to the new CPU limit on update of RolloutManager CR")

})
})
})
}