diff --git a/api/v1/infinispan_types.go b/api/v1/infinispan_types.go index 80679448b..69905f41d 100644 --- a/api/v1/infinispan_types.go +++ b/api/v1/infinispan_types.go @@ -386,6 +386,18 @@ type InfinispanExternalDependencies struct { VolumeClaimName string `json:"volumeClaimName,omitempty"` // +optional Artifacts []InfinispanExternalArtifacts `json:"artifacts,omitempty"` + // +optional + InitContainer InitDependenciesContainerSpec `json:"initContainer,omitempty"` +} + +// InitDependenciesContainerSpec describes the configuration options for the dependency download init container +type InitDependenciesContainerSpec struct { + // +optional + // Memory in limit:request format + Memory string `json:"memory,omitempty"` + // +optional + // CPU in limit:request format + CPU string `json:"cpu,omitempty"` } // ExternalArtifactType defines external artifact file type diff --git a/api/v1/infinispan_webhook.go b/api/v1/infinispan_webhook.go index 1b5c794ec..016f9f51b 100644 --- a/api/v1/infinispan_webhook.go +++ b/api/v1/infinispan_webhook.go @@ -239,29 +239,19 @@ func (i *Infinispan) validate() error { eventRec.Event(i, corev1.EventTypeWarning, "DeprecatedOperandVersion", msg) } - if i.Spec.Container.CPU != "" { - req, limit, err := i.Spec.Container.GetCpuResources() - if err != nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("container").Child("cpu"), i.Spec.Container.CPU, err.Error())) - } - - if req.Cmp(limit) > 0 { - msg := fmt.Sprintf("CPU request '%s' exceeds limit '%s'", req.String(), limit.String()) - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("container").Child("cpu"), i.Spec.Container.CPU, msg)) - } - } - - memReq, memLimit, err := i.Spec.Container.GetMemoryResources() - if i.Spec.Container.Memory != "" { - if err != nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("container").Child("memory"), i.Spec.Container.Memory, err.Error())) - } - - if memReq.Cmp(memLimit) > 0 { - msg := fmt.Sprintf("Memory request '%s' exceeds limit '%s'", memReq.String(), memLimit.String()) - allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("container").Child("memory"), i.Spec.Container.Memory, msg)) - } - } + validateRequestLimits( + i.Spec.Container.CPU, + i.Spec.Container.GetCpuResources, + field.NewPath("spec").Child("container").Child("cpu"), + &allErrs, + ) + + _, memLimit := validateRequestLimits( + i.Spec.Container.Memory, + i.Spec.Container.GetMemoryResources, + field.NewPath("spec").Child("container").Child("memory"), + &allErrs, + ) // Warn if memory size exceeds persistent vol if i.IsDataGrid() && !i.IsEphemeralStorage() && i.StorageSize() != "" { @@ -345,14 +335,29 @@ func (i *Infinispan) validate() error { } if i.HasExternalArtifacts() { + path := field.NewPath("spec").Child("dependencies") for i, artifact := range i.Spec.Dependencies.Artifacts { - f := field.NewPath("spec").Child("dependencies").Child("artifacts").Index(i) + f := path.Child("artifacts").Index(i) if artifact.Url == "" && artifact.Maven == "" { allErrs = append(allErrs, field.Required(f, "'artifact.Url' OR 'artifact.Maven' must be supplied")) } else if artifact.Url != "" && artifact.Maven != "" { allErrs = append(allErrs, field.Duplicate(f, "At most one of ['artifact.Url', 'artifact.Maven'] must be configured")) } } + + validateRequestLimits( + i.Spec.Dependencies.InitContainer.CPU, + i.Spec.Dependencies.InitContainer.CpuResources, + path.Child("initContainer").Child("cpu"), + &allErrs, + ) + + validateRequestLimits( + i.Spec.Dependencies.InitContainer.Memory, + i.Spec.Dependencies.InitContainer.MemoryResources, + path.Child("initContainer").Child("memory"), + &allErrs, + ) } if i.IsEphemeralStorage() { @@ -460,6 +465,23 @@ func (i *Infinispan) validate() error { return errorListToError(i, allErrs) } +func validateRequestLimits(val string, fn func() (req, limit resource.Quantity, err error), path *field.Path, allErrs *field.ErrorList) (req resource.Quantity, limit resource.Quantity) { + if val == "" { + return + } + + req, limit, err := fn() + if err != nil { + *allErrs = append(*allErrs, field.Invalid(path, val, err.Error())) + } + + if req.Cmp(limit) > 0 { + msg := fmt.Sprintf("Request '%s' exceeds limit '%s'", req.String(), limit.String()) + *allErrs = append(*allErrs, field.Invalid(path, val, msg)) + } + return +} + func errorListToError(i *Infinispan, allErrs field.ErrorList) error { if len(allErrs) != 0 { return apierrors.NewInvalid( diff --git a/api/v1/infinispan_webhook_test.go b/api/v1/infinispan_webhook_test.go index 6c9e65a0b..4b4cc13c7 100644 --- a/api/v1/infinispan_webhook_test.go +++ b/api/v1/infinispan_webhook_test.go @@ -283,6 +283,15 @@ var _ = Describe("Infinispan Webhooks", func() { Memory: "1Gi:5Gi", CPU: "1000m:2000m", }, + Dependencies: &InfinispanExternalDependencies{ + Artifacts: []InfinispanExternalArtifacts{ + {Maven: "org.example:dependency:1.0.0"}, + }, + InitContainer: InitDependenciesContainerSpec{ + Memory: "1Gi:5Gi", + CPU: "1000m:2000m", + }, + }, Service: InfinispanServiceSpec{ Type: ServiceTypeDataGrid, Sites: &InfinispanSitesSpec{ @@ -310,6 +319,10 @@ var _ = Describe("Infinispan Webhooks", func() { metav1.CauseTypeFieldValueInvalid, "spec.configListener.cpu", "exceeds limit", }, { metav1.CauseTypeFieldValueInvalid, "spec.configListener.memory", "exceeds limit", + }, { + metav1.CauseTypeFieldValueInvalid, "spec.dependencies.initContainer.cpu", "exceeds limit", + }, { + metav1.CauseTypeFieldValueInvalid, "spec.dependencies.initContainer.memory", "exceeds limit", }, { metav1.CauseTypeFieldValueInvalid, "spec.service.sites.local.discovery.cpu", "exceeds limit", }, { diff --git a/api/v1/types_util.go b/api/v1/types_util.go index 08d14c8c3..a81c7ee30 100644 --- a/api/v1/types_util.go +++ b/api/v1/types_util.go @@ -474,6 +474,16 @@ func (spec *DiscoverySiteSpec) MemoryResources() (requests resource.Quantity, li return getRequestLimits(spec.Memory) } +// CpuResources returns the CPU request and limit values to be used by by External Dependencies Downloader Init container +func (spec *InitDependenciesContainerSpec) CpuResources() (requests resource.Quantity, limits resource.Quantity, err error) { + return getRequestLimits(spec.CPU) +} + +// MemoryResources returns the Memory request and limit values to be used by External Dependencies Downloader Init container +func (spec *InitDependenciesContainerSpec) MemoryResources() (requests resource.Quantity, limits resource.Quantity, err error) { + return getRequestLimits(spec.Memory) +} + func getRequestLimits(str string) (requests resource.Quantity, limits resource.Quantity, err error) { if str == "" { err = fmt.Errorf("resource string cannot be empty") diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index d629ef78e..b02581f0a 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -448,6 +448,7 @@ func (in *InfinispanExternalDependencies) DeepCopyInto(out *InfinispanExternalDe *out = make([]InfinispanExternalArtifacts, len(*in)) copy(*out, *in) } + out.InitContainer = in.InitContainer } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InfinispanExternalDependencies. @@ -797,6 +798,21 @@ func (in *InfinispanUpgradesSpec) DeepCopy() *InfinispanUpgradesSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *InitDependenciesContainerSpec) DeepCopyInto(out *InitDependenciesContainerSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InitDependenciesContainerSpec. +func (in *InitDependenciesContainerSpec) DeepCopy() *InitDependenciesContainerSpec { + if in == nil { + return nil + } + out := new(InitDependenciesContainerSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JmxSpec) DeepCopyInto(out *JmxSpec) { *out = *in diff --git a/config/crd/bases/infinispan.org_infinispans.yaml b/config/crd/bases/infinispan.org_infinispans.yaml index 6f18854ac..b51da6382 100644 --- a/config/crd/bases/infinispan.org_infinispans.yaml +++ b/config/crd/bases/infinispan.org_infinispans.yaml @@ -975,6 +975,17 @@ spec: type: string type: object type: array + initContainer: + description: InitDependenciesContainerSpec describes the configuration + options for the dependency download init container + properties: + cpu: + description: CPU in limit:request format + type: string + memory: + description: Memory in limit:request format + type: string + type: object volumeClaimName: description: The Persistent Volume Claim that holds custom libraries type: string diff --git a/pkg/kubernetes/pods.go b/pkg/kubernetes/pods.go index 418fd73ee..54481a1fc 100644 --- a/pkg/kubernetes/pods.go +++ b/pkg/kubernetes/pods.go @@ -153,6 +153,15 @@ func GetContainer(name string, spec *corev1.PodSpec) *corev1.Container { return nil } +func GetInitContainer(name string, spec *corev1.PodSpec) *corev1.Container { + for i, c := range spec.InitContainers { + if c.Name == name { + return &spec.InitContainers[i] + } + } + return nil +} + func GetPodMemoryLimitBytes(container, podName, namespace string, kube *Kubernetes) (uint64, error) { execOut, err := kube.ExecWithOptions(ExecOptions{ Container: container, diff --git a/pkg/reconcile/pipeline/infinispan/handler/provision/dependencies.go b/pkg/reconcile/pipeline/infinispan/handler/provision/dependencies.go index 1986994d5..ae05afe72 100644 --- a/pkg/reconcile/pipeline/infinispan/handler/provision/dependencies.go +++ b/pkg/reconcile/pipeline/infinispan/handler/provision/dependencies.go @@ -1,6 +1,8 @@ package provision import ( + "fmt" + "reflect" "strings" ispnv1 "github.com/infinispan/infinispan-operator/api/v1" @@ -38,15 +40,26 @@ func ApplyExternalArtifactsDownload(ispn *ispnv1.Infinispan, ispnContainer *core volumes := &spec.Volumes volumeMounts := &ispnContainer.VolumeMounts containerPosition := kube.ContainerIndex(*initContainers, ExternalArtifactsDownloadInitContainer) + if ispn.HasExternalArtifacts() { + initContainerResources, err := getInitContainerResources(ispn) + if err != nil { + return false, fmt.Errorf("unable to calculate dependencies initContainer resources: %w", err) + } + serverLibs := serverLibs(ispn) if containerPosition >= 0 { if spec.InitContainers[containerPosition].Env[0].Value != serverLibs { spec.InitContainers[containerPosition].Env[0].Value = serverLibs updated = true } + if !reflect.DeepEqual(&spec.InitContainers[containerPosition].Resources, initContainerResources) { + spec.InitContainers[containerPosition].Resources = *initContainerResources + updated = true + } + } else { - *initContainers = append(*initContainers, corev1.Container{ + container := corev1.Container{ Image: ispn.ImageName(), Name: ExternalArtifactsDownloadInitContainer, Env: []corev1.EnvVar{ @@ -59,7 +72,11 @@ func ApplyExternalArtifactsDownload(ispn *ispnv1.Infinispan, ispnContainer *core Name: ExternalArtifactsVolumeName, MountPath: ExternalArtifactsMountPath, }}, - }) + } + if initContainerResources != nil { + container.Resources = *initContainerResources + } + *initContainers = append(*initContainers, container) *volumeMounts = append(*volumeMounts, corev1.VolumeMount{Name: ExternalArtifactsVolumeName, MountPath: ExternalArtifactsMountPath, ReadOnly: true}) *volumes = append(*volumes, corev1.Volume{Name: ExternalArtifactsVolumeName, VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}) updated = true @@ -75,6 +92,41 @@ func ApplyExternalArtifactsDownload(ispn *ispnv1.Infinispan, ispnContainer *core return } +func getInitContainerResources(i *ispnv1.Infinispan) (*corev1.ResourceRequirements, error) { + if i.Spec.Dependencies == nil { + return nil, nil + } + + spec := i.Spec.Dependencies.InitContainer + if spec.CPU == "" && spec.Memory == "" { + return &corev1.ResourceRequirements{}, nil + } + + req := &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{}, + Requests: corev1.ResourceList{}, + } + + if spec.Memory != "" { + memRequests, memLimits, err := spec.MemoryResources() + if err != nil { + return req, err + } + req.Requests[corev1.ResourceMemory] = memRequests + req.Limits[corev1.ResourceMemory] = memLimits + } + + if spec.CPU != "" { + cpuRequests, cpuLimits, err := spec.CpuResources() + if err != nil { + return req, err + } + req.Requests[corev1.ResourceCPU] = cpuRequests + req.Limits[corev1.ResourceCPU] = cpuLimits + } + return req, nil +} + func serverLibs(i *ispnv1.Infinispan) string { if !i.HasExternalArtifacts() { return "" diff --git a/test/e2e/infinispan/dependencies_test.go b/test/e2e/infinispan/dependencies_test.go index 24de5dab0..073922d0e 100644 --- a/test/e2e/infinispan/dependencies_test.go +++ b/test/e2e/infinispan/dependencies_test.go @@ -12,9 +12,12 @@ import ( ispnv1 "github.com/infinispan/infinispan-operator/api/v1" "github.com/infinispan/infinispan-operator/pkg/hash" kube "github.com/infinispan/infinispan-operator/pkg/kubernetes" + "github.com/infinispan/infinispan-operator/pkg/reconcile/pipeline/infinispan/handler/provision" tutils "github.com/infinispan/infinispan-operator/test/e2e/utils" + "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,14 +42,25 @@ func TestExternalDependenciesHttp(t *testing.T) { {Url: fmt.Sprintf("http://%s:%d/task01-1.0.0.jar", tutils.WebServerName, tutils.WebServerPortNumber)}, {Url: fmt.Sprintf("http://%s:%d/task02-1.0.0.zip", tutils.WebServerName, tutils.WebServerPortNumber)}, }, + InitContainer: ispnv1.InitDependenciesContainerSpec{ + Memory: "512Mi:64Mi", + CPU: "900m:100m", + }, } }) // Create the cluster testKube.CreateInfinispan(spec, tutils.Namespace) - testKube.WaitForInfinispanPods(1, tutils.SinglePodTimeout, spec.Name, namespace) + pod := testKube.WaitForInfinispanPods(1, tutils.SinglePodTimeout, spec.Name, namespace).Items[0] ispn := testKube.WaitForInfinispanCondition(spec.Name, spec.Namespace, ispnv1.ConditionWellFormed) + // Assert that the initContainer has the specified re + container := kube.GetInitContainer(provision.ExternalArtifactsDownloadInitContainer, &pod.Spec) + assert.Equal(t, resource.MustParse("512Mi"), *container.Resources.Limits.Memory()) + assert.Equal(t, resource.MustParse("64Mi"), *container.Resources.Requests.Memory()) + assert.Equal(t, resource.MustParse("900m"), *container.Resources.Limits.Cpu()) + assert.Equal(t, resource.MustParse("100m"), *container.Resources.Requests.Cpu()) + client_ := tutils.HTTPClientForCluster(ispn, testKube) validateTaskExecution := func(task, param string, status int, result string) { @@ -72,6 +86,23 @@ func TestExternalDependenciesHttp(t *testing.T) { validateTaskExecution("task-"+task, "World", http.StatusOK, "Hello World") } + // Ensure that updates to InitContainer resources result in a StatefulSet rollout and the appropriate values are configured + verifyStatefulSetUpdate( + *ispn, + func(ispn *ispnv1.Infinispan) { + ispn.Spec.Dependencies.InitContainer.CPU = "" + ispn.Spec.Dependencies.InitContainer.Memory = "" + }, + func(ispn *ispnv1.Infinispan, ss *appsv1.StatefulSet) { + testKube.WaitForInfinispanCondition(ispn.Name, ispn.Namespace, ispnv1.ConditionWellFormed) + container = kube.GetInitContainer(provision.ExternalArtifactsDownloadInitContainer, &ss.Spec.Template.Spec) + assert.True(t, container.Resources.Limits.Memory().IsZero()) + assert.True(t, container.Resources.Requests.Memory().IsZero()) + assert.True(t, container.Resources.Limits.Cpu().IsZero()) + assert.True(t, container.Resources.Requests.Cpu().IsZero()) + }, + ) + var externalLibraryAddModify = func(ispn *ispnv1.Infinispan) { libs := &ispn.Spec.Dependencies.Artifacts *libs = append(*libs, ispnv1.InfinispanExternalArtifacts{Url: fmt.Sprintf("http://%s:%d/task03-1.0.0.tar.gz", tutils.WebServerName, tutils.WebServerPortNumber)})