diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go index e367d1fd..9b517a9f 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile.go +++ b/pkg/controller/jenkins/configuration/base/reconcile.go @@ -718,7 +718,7 @@ func (r *ReconcileJenkinsBaseConfiguration) compareContainers(expected corev1.Co messages = append(messages, "Readiness probe has changed") verbose = append(verbose, fmt.Sprintf("Readiness probe has changed to '%+v' in container '%s'", expected.ReadinessProbe, expected.Name)) } - if !reflect.DeepEqual(expected.Resources, actual.Resources) { + if !compareContainerResources(expected.Resources, actual.Resources) { messages = append(messages, "Resources have changed") verbose = append(verbose, fmt.Sprintf("Resources have changed to '%+v' in container '%s'", expected.Resources, expected.Name)) } @@ -738,6 +738,33 @@ func (r *ReconcileJenkinsBaseConfiguration) compareContainers(expected corev1.Co return messages, verbose } +func compareContainerResources(expected corev1.ResourceRequirements, actual corev1.ResourceRequirements) bool { + expectedRequestCPU, expectedRequestCPUSet := expected.Requests[corev1.ResourceCPU] + expectedRequestMemory, expectedRequestMemorySet := expected.Requests[corev1.ResourceMemory] + expectedLimitCPU, expectedLimitCPUSet := expected.Limits[corev1.ResourceCPU] + expectedLimitMemory, expectedLimitMemorySet := expected.Limits[corev1.ResourceMemory] + + actualRequestCPU, actualRequestCPUSet := actual.Requests[corev1.ResourceCPU] + actualRequestMemory, actualRequestMemorySet := actual.Requests[corev1.ResourceMemory] + actualLimitCPU, actualLimitCPUSet := actual.Limits[corev1.ResourceCPU] + actualLimitMemory, actualLimitMemorySet := actual.Limits[corev1.ResourceMemory] + + if expectedRequestCPUSet && (!actualRequestCPUSet || expectedRequestCPU.String() != actualRequestCPU.String()) { + return false + } + if expectedRequestMemorySet && (!actualRequestMemorySet || expectedRequestMemory.String() != actualRequestMemory.String()) { + return false + } + if expectedLimitCPUSet && (!actualLimitCPUSet || expectedLimitCPU.String() != actualLimitCPU.String()) { + return false + } + if expectedLimitMemorySet && (!actualLimitMemorySet || expectedLimitMemory.String() != actualLimitMemory.String()) { + return false + } + + return true +} + func compareImagePullSecrets(expected, actual []corev1.LocalObjectReference) bool { for _, expected := range expected { found := false diff --git a/pkg/controller/jenkins/configuration/base/reconcile_test.go b/pkg/controller/jenkins/configuration/base/reconcile_test.go index 8a4bdb9e..e4c1e92b 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile_test.go +++ b/pkg/controller/jenkins/configuration/base/reconcile_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" 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" "k8s.io/client-go/kubernetes/scheme" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -920,3 +921,191 @@ func TestEnsureExtraRBAC(t *testing.T) { assert.Equal(t, jenkins.Spec.Roles[0], roleBindings.Items[2].RoleRef) }) } + +func TestCompareContainerResources(t *testing.T) { + t.Run("empty", func(t *testing.T) { + var expected corev1.ResourceRequirements + var actual corev1.ResourceRequirements + + got := compareContainerResources(expected, actual) + + assert.True(t, got) + }) + t.Run("expected resources empty, actual resources set", func(t *testing.T) { + var expected corev1.ResourceRequirements + actual := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("100Mi"), + }, + } + + got := compareContainerResources(expected, actual) + + assert.True(t, got) + }) + t.Run("request CPU the same values", func(t *testing.T) { + actual := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + }, + } + expected := actual + + got := compareContainerResources(expected, actual) + + assert.True(t, got) + }) + t.Run("request memory the same values", func(t *testing.T) { + actual := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + } + expected := actual + + got := compareContainerResources(expected, actual) + + assert.True(t, got) + }) + t.Run("limit CPU the same values", func(t *testing.T) { + actual := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + }, + } + expected := actual + + got := compareContainerResources(expected, actual) + + assert.True(t, got) + }) + t.Run("limit memory the same values", func(t *testing.T) { + actual := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + } + expected := actual + + got := compareContainerResources(expected, actual) + + assert.True(t, got) + }) + t.Run("request CPU different values", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5m"), + }, + } + actual := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + }, + } + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("request memory different values", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Mi"), + }, + } + actual := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + } + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("limit CPU different values", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5m"), + }, + } + actual := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + }, + } + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("limit memory different values", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Mi"), + }, + } + actual := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + } + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("request CPU not set in actual", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5m"), + }, + } + var actual corev1.ResourceRequirements + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("request memory not set in actual", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Mi"), + }, + } + var actual corev1.ResourceRequirements + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("limit CPU not set in actual", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("5m"), + }, + } + var actual corev1.ResourceRequirements + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) + t.Run("limit memory not set in actual", func(t *testing.T) { + expected := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Mi"), + }, + } + var actual corev1.ResourceRequirements + + got := compareContainerResources(expected, actual) + + assert.False(t, got) + }) +} diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index b564dfc8..151af31e 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -540,12 +540,7 @@ func setDefaultsForContainer(jenkins *v1alpha2.Jenkins, containerIndex int, logg } func isResourceRequirementsNotSet(requirements corev1.ResourceRequirements) bool { - _, requestCPUSet := requirements.Requests[corev1.ResourceCPU] - _, requestMemorySet := requirements.Requests[corev1.ResourceMemory] - _, limitCPUSet := requirements.Limits[corev1.ResourceCPU] - _, limitMemorySet := requirements.Limits[corev1.ResourceMemory] - - return !limitCPUSet || !limitMemorySet || !requestCPUSet || !requestMemorySet + return reflect.DeepEqual(requirements, corev1.ResourceRequirements{}) } func basePlugins() (result []v1alpha2.Plugin) {