Skip to content

Commit

Permalink
Container envs improvements
Browse files Browse the repository at this point in the history 
> possibility to enforce workdir
> add TTL to submission jobs (for cleanup and automatic retrigger of submission)
  • Loading branch information
@QcFe
QcFe committed Sep 19, 2022
1 parent efe3506 commit cedc13c
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 74 deletions.
4 changes: 4 additions & 0 deletions operators/api/v1alpha2/template_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ type ContainerStartupOpts struct {
ContentPath string `json:"contentPath,omitempty"`
// Arguments to be passed to the application container on startup
StartupArgs []string `json:"startupArgs,omitempty"`
// Whether forcing the container working directory to be the same as the contentPath (or default mydrive path if not specified)
// +kubebuilder:validation:Optional
// +kubebuilder:default=false
EnforceWorkdir bool `json:"enforceWorkdir"`
}

// +kubebuilder:object:root=true
Expand Down
6 changes: 6 additions & 0 deletions operators/deploy/crds/crownlabs.polito.it_templates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ spec:
be mounted and into which, if given in SourceArchiveURL,
will be extracted the archive
type: string
enforceWorkdir:
default: false
description: Whether forcing the container working directory
to be the same as the contentPath (or default mydrive
path if not specified)
type: boolean
sourceArchiveURL:
description: URL from which GET the archive to be extracted
into ContentPath
Expand Down
23 changes: 9 additions & 14 deletions operators/pkg/forge/containers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const (
CrownLabsUserID = int64(1010)
// SubmissionJobMaxRetries -> max number of retries for submission jobs.
SubmissionJobMaxRetries = 10
// SubmissionJobTTLSeconds -> seconds for submission jobs before deletion (either failure or success).
SubmissionJobTTLSeconds = 300
// AppCPULimitsEnvName -> name of the env variable containing AppContainer CPU limits.
AppCPULimitsEnvName = "APP_CPU_LIMITS"
// AppMEMLimitsEnvName -> name of the env variable containing AppContainer memory limits.
Expand Down Expand Up @@ -159,7 +161,8 @@ func PodSpec(instance *clv1alpha2.Instance, environment *clv1alpha2.Environment,
// SubmissionJobSpec returns the job spec for the submission job.
func SubmissionJobSpec(instance *clv1alpha2.Instance, environment *clv1alpha2.Environment, opts *ContainerEnvOpts) batchv1.JobSpec {
return batchv1.JobSpec{
BackoffLimit: pointer.Int32(SubmissionJobMaxRetries),
BackoffLimit: pointer.Int32(SubmissionJobMaxRetries),
TTLSecondsAfterFinished: pointer.Int32(SubmissionJobTTLSeconds),
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand Down Expand Up @@ -209,7 +212,7 @@ func WebsockifyContainer(opts *ContainerEnvOpts, environment *clv1alpha2.Environ
AddContainerArg(&websockifyContainer, "pod-name", fmt.Sprintf("$(%s)", PodNameEnvName))
AddContainerArg(&websockifyContainer, "cpu-limit", fmt.Sprintf("$(%s)", AppCPULimitsEnvName))
AddContainerArg(&websockifyContainer, "memory-limit", fmt.Sprintf("$(%s)", AppMEMLimitsEnvName))
SetContainerReadinessTCPProbe(&websockifyContainer, GUIPortName)
SetContainerReadinessHTTPProbe(&websockifyContainer, GUIPortName, IngressGUICleanPath(instance))
return websockifyContainer
}

Expand Down Expand Up @@ -260,11 +263,12 @@ func AppContainer(instance *clv1alpha2.Instance, environment *clv1alpha2.Environ
SetContainerResourcesFromEnvironment(&appContainer, environment)
AddEnvVariableFromResourcesToContainer(&appContainer, "CROWNLABS_CPU_REQUESTS", appContainer.Name, corev1.ResourceRequestsCPU, DefaultDivisor)
AddEnvVariableFromResourcesToContainer(&appContainer, "CROWNLABS_CPU_LIMITS", appContainer.Name, corev1.ResourceLimitsCPU, DefaultDivisor)
if NeedsContainerVolume(instance, environment) {
AddContainerVolumeMount(&appContainer, MyDriveName, volumeMountPath)
}
AddContainerVolumeMount(&appContainer, MyDriveName, volumeMountPath)
if environment.ContainerStartupOptions != nil {
appContainer.Args = environment.ContainerStartupOptions.StartupArgs
if environment.ContainerStartupOptions.EnforceWorkdir {
appContainer.WorkingDir = MyDriveMountPath(environment)
}
}
return appContainer
}
Expand Down Expand Up @@ -439,9 +443,6 @@ func SetContainerResourcesFromEnvironment(c *corev1.Container, env *clv1alpha2.E
// ContainerVolumes forges the list of volumes for the deployment spec, possibly returning an empty
// list in case the environment is not standard and not persistent.
func ContainerVolumes(instance *clv1alpha2.Instance, environment *clv1alpha2.Environment) []corev1.Volume {
if !NeedsContainerVolume(instance, environment) {
return nil
}
return []corev1.Volume{ContainerVolume(MyDriveName, NamespacedName(instance).Name, environment)}
}

Expand All @@ -467,12 +468,6 @@ func ContainerVolume(volumeName, claimName string, environment *clv1alpha2.Envir
}
}

// NeedsContainerVolume returns true in the cases in which a volume mount could be needed.
func NeedsContainerVolume(instance *clv1alpha2.Instance, environment *clv1alpha2.Environment) bool {
needsInit, _ := NeedsInitContainer(instance, environment)
return environment.Mode == clv1alpha2.ModeStandard || environment.Persistent || needsInit
}

// NeedsInitContainer returns true if the environment requires an initcontainer in order to be prepopulated.
func NeedsInitContainer(instance *clv1alpha2.Instance, environment *clv1alpha2.Environment) (value bool, contentOrigin string) {
if icu := instance.Spec.CustomizationUrls; icu != nil && icu.ContentOrigin != "" {
Expand Down
106 changes: 46 additions & 60 deletions operators/pkg/forge/containers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ var _ = Describe("Containers and Deployment spec forging", func() {
Expect(actual.Ports).To(Equal(expected.Ports))
})
It("Should set the readiness probe", func() {
forge.SetContainerReadinessTCPProbe(&expected, "gui")
forge.SetContainerReadinessHTTPProbe(&expected, "gui", forge.IngressGUICleanPath(&instance))
Expect(actual.ReadinessProbe).To(Equal(expected.ReadinessProbe))
})
It("Should set the env varibles", func() {
Expand Down Expand Up @@ -574,6 +574,39 @@ var _ = Describe("Containers and Deployment spec forging", func() {
ExpectedOutput: func(e *clv1alpha2.Environment) []string { return testArguments },
}))
})

Context("Has to handle the workdir", func() {
type ContainerCase struct {
StartupOpts *clv1alpha2.ContainerStartupOpts
ExpectedOutput func(*clv1alpha2.Environment) string
}

WhenBody := func(c ContainerCase) func() {
return func() {
BeforeEach(func() {
environment.ContainerStartupOptions = c.StartupOpts
})

JustBeforeEach(func() {
actual = forge.AppContainer(&instance, &environment, myDriveMountPath)
})

It("Should set the WorkingDirectory accordingly", func() {
Expect(actual.WorkingDir).To(Equal(c.ExpectedOutput(&environment)))
})
}
}

When("ContainerStartupOptions is nil", WhenBody(ContainerCase{
StartupOpts: nil,
ExpectedOutput: func(e *clv1alpha2.Environment) string { return "" },
}))

When("EnforceWorkdir is set", WhenBody(ContainerCase{
StartupOpts: &clv1alpha2.ContainerStartupOpts{EnforceWorkdir: true},
ExpectedOutput: forge.MyDriveMountPath,
}))
})
})

Describe("The forge.InitContainers function forges the list of init containers for the podSpec", func() {
Expand Down Expand Up @@ -671,7 +704,8 @@ var _ = Describe("Containers and Deployment spec forging", func() {

It("should return the correct podSpecification", func() {
Expect(actual).To(Equal(batchv1.JobSpec{
BackoffLimit: pointer.Int32Ptr(forge.SubmissionJobMaxRetries),
BackoffLimit: pointer.Int32Ptr(forge.SubmissionJobMaxRetries),
TTLSecondsAfterFinished: pointer.Int32Ptr(forge.SubmissionJobTTLSeconds),
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand Down Expand Up @@ -940,15 +974,19 @@ var _ = Describe("Containers and Deployment spec forging", func() {
}))

When("the environment is not persistent and mode is exam", WhenBody(ContainerVolumesCase{
Persistent: false,
Mode: clv1alpha2.ModeExam,
ExpectedOutputVSs: func(e *clv1alpha2.Environment) []corev1.Volume { return nil },
Persistent: false,
Mode: clv1alpha2.ModeExam,
ExpectedOutputVSs: func(e *clv1alpha2.Environment) []corev1.Volume {
return []corev1.Volume{forge.ContainerVolume(myDriveName, instanceName, e)}
},
}))

When("the environment is not persistent and mode is exercise", WhenBody(ContainerVolumesCase{
Persistent: false,
Mode: clv1alpha2.ModeExercise,
ExpectedOutputVSs: func(e *clv1alpha2.Environment) []corev1.Volume { return nil },
Persistent: false,
Mode: clv1alpha2.ModeExercise,
ExpectedOutputVSs: func(e *clv1alpha2.Environment) []corev1.Volume {
return []corev1.Volume{forge.ContainerVolume(myDriveName, instanceName, e)}
},
}))

When("the environment is persistent and mode is standard", WhenBody(ContainerVolumesCase{
Expand Down Expand Up @@ -1031,58 +1069,6 @@ var _ = Describe("Containers and Deployment spec forging", func() {
}))
})

Describe("The forge.NeedsContainerVolume function", func() {
type NeedsContainerVolumeCase struct {
StartupOpts *clv1alpha2.ContainerStartupOpts
Persistent bool
Mode clv1alpha2.EnvironmentMode
ExpectedOutput bool
}

WhenBody := func(c NeedsContainerVolumeCase) func() {
return func() {
BeforeEach(func() {
environment.ContainerStartupOptions = c.StartupOpts
environment.Persistent = c.Persistent
environment.Mode = c.Mode
})

It("Should return the correct value", func() {
Expect(forge.NeedsContainerVolume(&instance, &environment)).To(Equal(c.ExpectedOutput))
})
}
}

When("the volume should not be needed", WhenBody(NeedsContainerVolumeCase{
StartupOpts: nil,
Persistent: false,
Mode: clv1alpha2.ModeExercise,
ExpectedOutput: false,
}))

When("the mode is standard", WhenBody(NeedsContainerVolumeCase{
StartupOpts: nil,
Persistent: false,
Mode: clv1alpha2.ModeStandard,
ExpectedOutput: true,
}))

When("the environment is persistent", WhenBody(NeedsContainerVolumeCase{
StartupOpts: nil,
Persistent: true,
Mode: clv1alpha2.ModeExercise,
ExpectedOutput: true,
}))

When("a source archive is specified", WhenBody(NeedsContainerVolumeCase{
StartupOpts: &clv1alpha2.ContainerStartupOpts{SourceArchiveURL: httpPath},
Persistent: false,
Mode: clv1alpha2.ModeExercise,
ExpectedOutput: true,
}))

})

Describe("The forge.NeedsInitContainer function", func() {
type NeedsInitContainerCase struct {
StartupOpts *clv1alpha2.ContainerStartupOpts
Expand Down

0 comments on commit cedc13c

Please sign in to comment.