Skip to content

Commit

Permalink
Operator can configure limit of staging MemoryMB
Browse files Browse the repository at this point in the history 
[#2654]

Reorganized the staging resource configuration under its own struct to
better capture the intent of the fields.

Co-authored-by: Dave Walter <[email protected]>
  • Loading branch information
@acosta11 @davewalter
acosta11 and davewalter committed Jul 11, 2023
1 parent 763151e commit 3f5953b
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 30 deletions.
41 changes: 23 additions & 18 deletions controllers/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,21 @@ type ControllerConfig struct {
IncludeContourRouter bool `yaml:"includeContourRouter"`

// core controllers
CFProcessDefaults CFProcessDefaults `yaml:"cfProcessDefaults"`
CFRootNamespace string `yaml:"cfRootNamespace"`
ContainerRegistrySecretNames []string `yaml:"containerRegistrySecretNames"`
TaskTTL string `yaml:"taskTTL"`
WorkloadsTLSSecretName string `yaml:"workloads_tls_secret_name"`
WorkloadsTLSSecretNamespace string `yaml:"workloads_tls_secret_namespace"`
BuilderName string `yaml:"builderName"`
RunnerName string `yaml:"runnerName"`
NamespaceLabels map[string]string `yaml:"namespaceLabels"`
ExtraVCAPApplicationValues map[string]any `yaml:"extraVCAPApplicationValues"`
MaxRetainedPackagesPerApp int `yaml:"maxRetainedPackagesPerApp"`
MaxRetainedBuildsPerApp int `yaml:"maxRetainedBuildsPerApp"`
LogLevel zapcore.Level `yaml:"logLevel"`
SpaceFinalizerAppDeletionTimeout *int64 `yaml:"spaceFinalizerAppDeletionTimeout"`
CFProcessDefaults CFProcessDefaults `yaml:"cfProcessDefaults"`
CFStagingResourceLimits CFStagingResourceLimits `yaml:"cfStagingResourceLimits"`
CFRootNamespace string `yaml:"cfRootNamespace"`
ContainerRegistrySecretNames []string `yaml:"containerRegistrySecretNames"`
TaskTTL string `yaml:"taskTTL"`
WorkloadsTLSSecretName string `yaml:"workloads_tls_secret_name"`
WorkloadsTLSSecretNamespace string `yaml:"workloads_tls_secret_namespace"`
BuilderName string `yaml:"builderName"`
RunnerName string `yaml:"runnerName"`
NamespaceLabels map[string]string `yaml:"namespaceLabels"`
ExtraVCAPApplicationValues map[string]any `yaml:"extraVCAPApplicationValues"`
MaxRetainedPackagesPerApp int `yaml:"maxRetainedPackagesPerApp"`
MaxRetainedBuildsPerApp int `yaml:"maxRetainedBuildsPerApp"`
LogLevel zapcore.Level `yaml:"logLevel"`
SpaceFinalizerAppDeletionTimeout *int64 `yaml:"spaceFinalizerAppDeletionTimeout"`

// job-task-runner
JobTTL string `yaml:"jobTTL"`
Expand All @@ -41,8 +42,6 @@ type ControllerConfig struct {
BuilderReadinessTimeout string `yaml:"builderReadinessTimeout"`
ContainerRepositoryPrefix string `yaml:"containerRepositoryPrefix"`
ContainerRegistryType string `yaml:"containerRegistryType"`
BuildCacheMB int `yaml:"buildCacheMB"`
DiskMB int `yaml:"diskMB"`
}

type CFProcessDefaults struct {
Expand All @@ -51,6 +50,12 @@ type CFProcessDefaults struct {
Timeout *int64 `yaml:"timeout"`
}

type CFStagingResourceLimits struct {
BuildCacheMB int64 `yaml:"buildCacheMB"`
DiskMB int64 `yaml:"diskMB"`
MemoryMB int64 `yaml:"memoryMB"`
}

const (
defaultTaskTTL = 30 * 24 * time.Hour
defaultTimeout int64 = 60
Expand All @@ -73,8 +78,8 @@ func LoadFromPath(path string) (*ControllerConfig, error) {
config.SpaceFinalizerAppDeletionTimeout = tools.PtrTo(defaultTimeout)
}

if config.BuildCacheMB == 0 {
config.BuildCacheMB = defaultBuildCacheMB
if config.CFStagingResourceLimits.BuildCacheMB == 0 {
config.CFStagingResourceLimits.BuildCacheMB = defaultBuildCacheMB
}

return &config, nil
Expand Down
18 changes: 12 additions & 6 deletions controllers/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ var _ = Describe("LoadFromPath", func() {
DiskQuotaMB: 512,
Timeout: tools.PtrTo(int64(30)),
},
CFStagingResourceLimits: config.CFStagingResourceLimits{
BuildCacheMB: 1024,
DiskMB: 512,
MemoryMB: 2048,
},
CFRootNamespace: "rootNamespace",
ContainerRegistrySecretNames: []string{"packageRegistrySecretName"},
TaskTTL: "taskTTL",
Expand All @@ -45,8 +50,6 @@ var _ = Describe("LoadFromPath", func() {
JobTTL: "jobTTL",
LogLevel: zapcore.DebugLevel,
SpaceFinalizerAppDeletionTimeout: tools.PtrTo(int64(42)),
BuildCacheMB: 1024,
DiskMB: 512,
}
})

Expand All @@ -70,6 +73,11 @@ var _ = Describe("LoadFromPath", func() {
DiskQuotaMB: 512,
Timeout: tools.PtrTo(int64(30)),
},
CFStagingResourceLimits: config.CFStagingResourceLimits{
BuildCacheMB: 1024,
DiskMB: 512,
MemoryMB: 2048,
},
CFRootNamespace: "rootNamespace",
ContainerRegistrySecretNames: []string{"packageRegistrySecretName"},
TaskTTL: "taskTTL",
Expand All @@ -82,8 +90,6 @@ var _ = Describe("LoadFromPath", func() {
JobTTL: "jobTTL",
LogLevel: zapcore.DebugLevel,
SpaceFinalizerAppDeletionTimeout: tools.PtrTo(int64(42)),
BuildCacheMB: 1024,
DiskMB: 512,
}))
})

Expand Down Expand Up @@ -119,11 +125,11 @@ var _ = Describe("LoadFromPath", func() {

When("the staging build cache size is not set", func() {
BeforeEach(func() {
cfg.BuildCacheMB = 0
cfg.CFStagingResourceLimits.BuildCacheMB = 0
})

It("uses the default", func() {
Expect(retConfig.BuildCacheMB).To(Equal(2048))
Expect(retConfig.CFStagingResourceLimits.BuildCacheMB).To(Equal(int64(2048)))
})
})
})
Expand Down
6 changes: 4 additions & 2 deletions helm/korifi/controllers/configmap.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ data:
builderReadinessTimeout: {{ required "builderReadinessTimeout is required" .Values.kpackImageBuilder.builderReadinessTimeout }}
containerRepositoryPrefix: {{ .Values.global.containerRepositoryPrefix | quote }}
builderServiceAccount: kpack-service-account
buildCacheMB: {{ .Values.api.lifecycle.stagingRequirements.buildCacheMB }}
diskMB: {{ .Values.api.lifecycle.stagingRequirements.diskMB }}
cfStagingResourceLimits:
buildCacheMB: {{ .Values.api.lifecycle.stagingRequirements.buildCacheMB }}
diskMB: {{ .Values.api.lifecycle.stagingRequirements.diskMB }}
memoryMB: {{ .Values.api.lifecycle.stagingRequirements.memoryMB }}
{{- if .Values.global.eksContainerRegistryRoleARN }}
containerRegistryType: "ECR"
{{- end }}
Expand Down
5 changes: 3 additions & 2 deletions kpack-image-builder/controllers/buildworkload_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ func (r *BuildWorkloadReconciler) reconcileKpackImage(
return err
}

cacheSize, err := resource.ParseQuantity(fmt.Sprintf("%dMi", r.controllerConfig.BuildCacheMB))
cacheSize, err := resource.ParseQuantity(fmt.Sprintf("%dMi", r.controllerConfig.CFStagingResourceLimits.BuildCacheMB))
if err != nil {
log.Info("failed to parse image cache size", "reason", err)
return err
Expand Down Expand Up @@ -704,7 +704,8 @@ func (r *BuildWorkloadReconciler) reconcileKpackImage(
Env: buildWorkload.Spec.Env,
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceEphemeralStorage: *resource.NewScaledQuantity(int64(r.controllerConfig.DiskMB), resource.Mega),
corev1.ResourceEphemeralStorage: *resource.NewScaledQuantity(int64(r.controllerConfig.CFStagingResourceLimits.DiskMB), resource.Mega),
corev1.ResourceMemory: *resource.NewScaledQuantity(int64(r.controllerConfig.CFStagingResourceLimits.MemoryMB), resource.Mega),
},
},
},
Expand Down
1 change: 1 addition & 0 deletions kpack-image-builder/controllers/buildworkload_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ var _ = Describe("BuildWorkloadReconciler", func() {
g.Expect(kpackImage.Spec.Build.Env).To(Equal(env))
g.Expect(kpackImage.Spec.Build.Services).To(BeEquivalentTo(services))
g.Expect(kpackImage.Spec.Build.Resources.Limits.StorageEphemeral().String()).To(Equal(fmt.Sprintf("%dM", 2048)))
g.Expect(kpackImage.Spec.Build.Resources.Limits.Memory().String()).To(Equal(fmt.Sprintf("%dM", 1234)))

g.Expect(kpackImage.Spec.Builder.Kind).To(Equal("ClusterBuilder"))
g.Expect(kpackImage.Spec.Builder.Name).To(Equal("cf-kpack-builder"))
Expand Down
7 changes: 5 additions & 2 deletions kpack-image-builder/controllers/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,11 @@ var _ = BeforeSuite(func() {
ClusterBuilderName: "cf-kpack-builder",
ContainerRepositoryPrefix: "image/registry/tag",
BuilderServiceAccount: "builder-service-account",
BuildCacheMB: 1024,
DiskMB: 2048,
CFStagingResourceLimits: config.CFStagingResourceLimits{
BuildCacheMB: 1024,
DiskMB: 2048,
MemoryMB: 1234,
},
}

imageRepoCreator = new(fake.RepositoryCreator)
Expand Down

0 comments on commit 3f5953b

Please sign in to comment.