From 553294ff1eb59f059a06644c0881d32a36b2d0eb Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 5 Jan 2021 13:53:40 -0500 Subject: [PATCH 01/14] Move lifecycle functions to separate package Move files in pkg/library to pkg/library/lifecycle to make adding more library functions cleaner. Signed-off-by: Angel Misevski --- controllers/controller/component/component_controller.go | 5 +++-- pkg/library/{ => lifecycle}/command.go | 2 +- pkg/library/{ => lifecycle}/common.go | 2 +- pkg/library/{ => lifecycle}/lifecycle.go | 2 +- pkg/library/{ => lifecycle}/lifecycle_test.go | 2 +- .../testdata/lifecycle/init_and_main_container.yaml | 0 .../{ => lifecycle}/testdata/lifecycle/no_events.yaml | 0 .../testdata/lifecycle/prestart_apply_command.yaml | 0 .../testdata/lifecycle/prestart_exec_command.yaml | 0 9 files changed, 7 insertions(+), 6 deletions(-) rename pkg/library/{ => lifecycle}/command.go (99%) rename pkg/library/{ => lifecycle}/common.go (96%) rename pkg/library/{ => lifecycle}/lifecycle.go (99%) rename pkg/library/{ => lifecycle}/lifecycle_test.go (99%) rename pkg/library/{ => lifecycle}/testdata/lifecycle/init_and_main_container.yaml (100%) rename pkg/library/{ => lifecycle}/testdata/lifecycle/no_events.yaml (100%) rename pkg/library/{ => lifecycle}/testdata/lifecycle/prestart_apply_command.yaml (100%) rename pkg/library/{ => lifecycle}/testdata/lifecycle/prestart_exec_command.yaml (100%) diff --git a/controllers/controller/component/component_controller.go b/controllers/controller/component/component_controller.go index 665529998..de715a886 100644 --- a/controllers/controller/component/component_controller.go +++ b/controllers/controller/component/component_controller.go @@ -17,6 +17,8 @@ import ( "errors" "fmt" + "github.com/devfile/devworkspace-operator/pkg/library/lifecycle" + "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/adaptor" @@ -36,7 +38,6 @@ import ( controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/library" ) var configMapDiffOpts = cmp.Options{ @@ -80,7 +81,7 @@ func (r *ComponentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return reconcile.Result{}, nil } - initContainers, mainComponents, err := library.GetInitContainers(devworkspace.DevWorkspaceTemplateSpecContent{ + initContainers, mainComponents, err := lifecycle.GetInitContainers(devworkspace.DevWorkspaceTemplateSpecContent{ Components: instance.Spec.Components, Commands: instance.Spec.Commands, Events: instance.Spec.Events, diff --git a/pkg/library/command.go b/pkg/library/lifecycle/command.go similarity index 99% rename from pkg/library/command.go rename to pkg/library/lifecycle/command.go index 4579deaa9..8aa78b07c 100644 --- a/pkg/library/command.go +++ b/pkg/library/lifecycle/command.go @@ -10,7 +10,7 @@ // Red Hat, Inc. - initial API and implementation // -package library +package lifecycle import ( "fmt" diff --git a/pkg/library/common.go b/pkg/library/lifecycle/common.go similarity index 96% rename from pkg/library/common.go rename to pkg/library/lifecycle/common.go index 435995070..86f179889 100644 --- a/pkg/library/common.go +++ b/pkg/library/lifecycle/common.go @@ -10,7 +10,7 @@ // Red Hat, Inc. - initial API and implementation // -package library +package lifecycle func listContains(query string, list []string) bool { for _, elem := range list { diff --git a/pkg/library/lifecycle.go b/pkg/library/lifecycle/lifecycle.go similarity index 99% rename from pkg/library/lifecycle.go rename to pkg/library/lifecycle/lifecycle.go index 4cbe85581..a77fd56c6 100644 --- a/pkg/library/lifecycle.go +++ b/pkg/library/lifecycle/lifecycle.go @@ -10,7 +10,7 @@ // Red Hat, Inc. - initial API and implementation // -package library +package lifecycle import ( "fmt" diff --git a/pkg/library/lifecycle_test.go b/pkg/library/lifecycle/lifecycle_test.go similarity index 99% rename from pkg/library/lifecycle_test.go rename to pkg/library/lifecycle/lifecycle_test.go index 0f6aff8ea..ac7fb2b2c 100644 --- a/pkg/library/lifecycle_test.go +++ b/pkg/library/lifecycle/lifecycle_test.go @@ -10,7 +10,7 @@ // Red Hat, Inc. - initial API and implementation // -package library +package lifecycle import ( "fmt" diff --git a/pkg/library/testdata/lifecycle/init_and_main_container.yaml b/pkg/library/lifecycle/testdata/lifecycle/init_and_main_container.yaml similarity index 100% rename from pkg/library/testdata/lifecycle/init_and_main_container.yaml rename to pkg/library/lifecycle/testdata/lifecycle/init_and_main_container.yaml diff --git a/pkg/library/testdata/lifecycle/no_events.yaml b/pkg/library/lifecycle/testdata/lifecycle/no_events.yaml similarity index 100% rename from pkg/library/testdata/lifecycle/no_events.yaml rename to pkg/library/lifecycle/testdata/lifecycle/no_events.yaml diff --git a/pkg/library/testdata/lifecycle/prestart_apply_command.yaml b/pkg/library/lifecycle/testdata/lifecycle/prestart_apply_command.yaml similarity index 100% rename from pkg/library/testdata/lifecycle/prestart_apply_command.yaml rename to pkg/library/lifecycle/testdata/lifecycle/prestart_apply_command.yaml diff --git a/pkg/library/testdata/lifecycle/prestart_exec_command.yaml b/pkg/library/lifecycle/testdata/lifecycle/prestart_exec_command.yaml similarity index 100% rename from pkg/library/testdata/lifecycle/prestart_exec_command.yaml rename to pkg/library/lifecycle/testdata/lifecycle/prestart_exec_command.yaml From f76d79bda25ee5922ca5b6ff787006bd55882db2 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 5 Jan 2021 13:54:51 -0500 Subject: [PATCH 02/14] Add library functions for converting Containers Add library function GetKubeContainersFromDevfile to convert container components in a flattened devfile into k8s Container objects. Signed-off-by: Angel Misevski --- pkg/library/container/container.go | 193 +++++++++++++++++++++++++++++ pkg/library/flatten.go | 27 ++++ 2 files changed, 220 insertions(+) create mode 100644 pkg/library/container/container.go create mode 100644 pkg/library/flatten.go diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go new file mode 100644 index 000000000..7d89db7f3 --- /dev/null +++ b/pkg/library/container/container.go @@ -0,0 +1,193 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +// Package container contains library functions for converting DevWorkspace Container components to Kubernetes +// components +package container + +import ( + "fmt" + + devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/library" + "github.com/devfile/devworkspace-operator/pkg/library/lifecycle" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +const ( + ProjectsRootEnvVar = "PROJECTS_ROOT" + ProjectsSourceEnvVar = "PROJECTS_SOURCE" + ProjecstVolumeName = "projects" +) + +// GetKubeContainersFromDevfile converts container components in a DevWorkspace into Kubernetes containers. +// If a DevWorkspace container is an init container (i.e. is bound to a preStart event), it will be returned as an +// init container. +// +// Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin) +func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpec) (*v1alpha1.PodAdditions, error) { + if !library.DevWorkspaceIsFlattened(workspace) { + return nil, fmt.Errorf("devfile is not flattened") + } + podAdditions := &v1alpha1.PodAdditions{} + + initContainers, mainComponents, err := lifecycle.GetInitContainers(workspace.DevWorkspaceTemplateSpecContent) + if err != nil { + return nil, err + } + + for _, component := range mainComponents { + if component.Container == nil { + continue + } + k8sContainer, err := convertContainerToK8s(component) + if err != nil { + return nil, err + } + podAdditions.Containers = append(podAdditions.Containers, *k8sContainer) + } + + for _, container := range initContainers { + k8sContainer, err := convertContainerToK8s(container) + if err != nil { + return nil, err + } + podAdditions.InitContainers = append(podAdditions.InitContainers, *k8sContainer) + } + + fillDefaultEnvVars(podAdditions, workspace) + + return podAdditions, nil +} + +func convertContainerToK8s(devfileComponent devworkspace.Component) (*corev1.Container, error) { + if devfileComponent.Container == nil { + return nil, fmt.Errorf("cannot get k8s container from non-container component") + } + devfileContainer := devfileComponent.Container + + containerResources, err := devfileResourcesToContainerResources(devfileContainer) + if err != nil { + return nil, err + } + + var mountSources bool + if devfileContainer.MountSources == nil { + mountSources = true + } else { + mountSources = *devfileContainer.MountSources + } + + container := &corev1.Container{ + Name: devfileComponent.Name, + Image: devfileContainer.Image, + Command: devfileContainer.Command, + Args: devfileContainer.Args, + Resources: *containerResources, + Ports: devfileEndpointsToContainerPorts(devfileContainer.Endpoints), + Env: devfileEnvToContainerEnv(devfileContainer.Env), + VolumeMounts: devfileVolumeMountsToContainerVolumeMounts(devfileContainer.VolumeMounts, mountSources), + ImagePullPolicy: corev1.PullPolicy(config.ControllerCfg.GetSidecarPullPolicy()), + } + + return container, nil +} + +func devfileEndpointsToContainerPorts(endpoints []devworkspace.Endpoint) []corev1.ContainerPort { + var containerPorts []corev1.ContainerPort + for _, endpoint := range endpoints { + containerPorts = append(containerPorts, corev1.ContainerPort{ + Name: endpoint.Name, + ContainerPort: int32(endpoint.TargetPort), + Protocol: corev1.ProtocolTCP, + }) + } + return containerPorts +} + +func devfileResourcesToContainerResources(devfileContainer *devworkspace.ContainerComponent) (*corev1.ResourceRequirements, error) { + // TODO: Handle memory request and CPU when implemented in devfile API + memLimit := devfileContainer.MemoryLimit + if memLimit == "" { + memLimit = config.SidecarDefaultMemoryLimit + } + memLimitQuantity, err := resource.ParseQuantity(memLimit) + if err != nil { + return nil, fmt.Errorf("failed to parse memory limit %q: %w", memLimit, err) + } + return &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: memLimitQuantity, + }, + }, nil +} + +func devfileVolumeMountsToContainerVolumeMounts(devfileVolumeMounts []devworkspace.VolumeMount, mountSources bool) []corev1.VolumeMount { + var volumeMounts []corev1.VolumeMount + for _, vm := range devfileVolumeMounts { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: vm.Name, + MountPath: vm.Path, + }) + } + if mountSources { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: ProjecstVolumeName, + MountPath: config.DefaultProjectsSourcesRoot, + }) + } + return volumeMounts +} + +func devfileEnvToContainerEnv(devfileEnvVars []devworkspace.EnvVar) []corev1.EnvVar { + var env []corev1.EnvVar + for _, devfileEnv := range devfileEnvVars { + env = append(env, corev1.EnvVar{ + Name: devfileEnv.Name, + Value: devfileEnv.Value, + }) + } + return env +} + +func fillDefaultEnvVars(podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) { + var projectsSource string + if len(workspace.Projects) > 0 { + // TODO: Unclear from devfile spec how this should work when there are multiple projects + projectsSource = fmt.Sprintf("%s/%s", config.DefaultProjectsSourcesRoot, workspace.Projects[0].ClonePath) + } else { + projectsSource = config.DefaultProjectsSourcesRoot + } + + // Add devfile reserved env var and legacy env var for Che-Theia + for idx, container := range podAdditions.Containers { + podAdditions.Containers[idx].Env = append(container.Env, corev1.EnvVar{ + Name: ProjectsRootEnvVar, + Value: config.DefaultProjectsSourcesRoot, + }, corev1.EnvVar{ + Name: ProjectsSourceEnvVar, + Value: projectsSource, + }) + } + for idx, container := range podAdditions.InitContainers { + podAdditions.InitContainers[idx].Env = append(container.Env, corev1.EnvVar{ + Name: ProjectsRootEnvVar, + Value: config.DefaultProjectsSourcesRoot, + }, corev1.EnvVar{ + Name: ProjectsSourceEnvVar, + Value: projectsSource, + }) + } +} diff --git a/pkg/library/flatten.go b/pkg/library/flatten.go new file mode 100644 index 000000000..fd6d6ae02 --- /dev/null +++ b/pkg/library/flatten.go @@ -0,0 +1,27 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package library + +import devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + +func DevWorkspaceIsFlattened(devworkspace devworkspace.DevWorkspaceTemplateSpec) bool { + if devworkspace.Parent != nil { + return false + } + for _, component := range devworkspace.Components { + if component.Plugin != nil { + return false + } + } + return true +} From 21778b9d8be409ccf0140d568b7699440d9354cd Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 5 Jan 2021 13:56:15 -0500 Subject: [PATCH 03/14] Add library function for provisioning 'common' PVC storage Add library function RewriteContainerVolumeMounts that converts containers and initContainers VolumeMounts to use a common PVC. Signed-off-by: Angel Misevski --- pkg/library/storage/commonStorage.go | 65 ++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 pkg/library/storage/commonStorage.go diff --git a/pkg/library/storage/commonStorage.go b/pkg/library/storage/commonStorage.go new file mode 100644 index 000000000..5fdb7da7f --- /dev/null +++ b/pkg/library/storage/commonStorage.go @@ -0,0 +1,65 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package storage + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + + devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/config" +) + +func RewriteContainerVolumeMounts(workspaceId string, podAdditions *v1alpha1.PodAdditions, devfile devworkspace.DevWorkspaceTemplateSpec) error { + devfileVolumes := map[string]devworkspace.VolumeComponent{} + for _, component := range devfile.Components { + if component.Volume != nil { + if _, exists := devfileVolumes[component.Name]; exists { + return fmt.Errorf("volume component %s is defined multiple times", component.Name) + } + devfileVolumes[component.Name] = *component.Volume + } + } + // TODO: Support more than the common PVC strategy here (storage provisioner interface?) + // TODO: What should we do when a volume isn't explicitly defined? + commonPVCName := config.ControllerCfg.GetWorkspacePVCName() + for cIdx, container := range podAdditions.Containers { + for vmIdx, vm := range container.VolumeMounts { + if _, ok := devfileVolumes[vm.Name]; !ok { + return fmt.Errorf("container %s references undefined volume %s", container.Name, vm.Name) + } + podAdditions.Containers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name) + podAdditions.Containers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName + } + } + for cIdx, container := range podAdditions.InitContainers { + for vmIdx, vm := range container.VolumeMounts { + if _, ok := devfileVolumes[vm.Name]; !ok { + return fmt.Errorf("container %s references undefined volume %s", container.Name, vm.Name) + } + podAdditions.InitContainers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name) + podAdditions.InitContainers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName + } + } + podAdditions.Volumes = append(podAdditions.Volumes, corev1.Volume{ + Name: commonPVCName, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: commonPVCName, + }, + }, + }) + return nil +} From 01c5ca8e044fdc36fc6b807bba8cd8b87f00b5a3 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Wed, 6 Jan 2021 16:34:33 -0500 Subject: [PATCH 04/14] Add library package for accomodating current Che Theia requirements Add package pkg/library/shim which is used to reconstruct ComponentMetadata as required by che-rest-apis (which is in turn required by Che-Theia). This package is necessary as a shim between purely devfile 2.0 devworkspaces and current devworkspaces, which use devfile 1.0 concepts (plugin meta.yaml, Theia's dependence on Che server, etc.). Eventually this library should be deprecated. Signed-off-by: Angel Misevski --- pkg/library/shim/component.go | 134 ++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 pkg/library/shim/component.go diff --git a/pkg/library/shim/component.go b/pkg/library/shim/component.go new file mode 100644 index 000000000..bf77c4752 --- /dev/null +++ b/pkg/library/shim/component.go @@ -0,0 +1,134 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +// package shim contains functions for generating metadata needed by Che-Theia for correct +// representation of workspaces. These functions serve to enable compatibility between devfile 2.0 +// workspaces and Che-Theia until Theia gets devfile 2.0 support. +package shim + +import ( + "fmt" + + "github.com/devfile/api/pkg/attributes" + "github.com/devfile/devworkspace-operator/pkg/config" + + devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +func GetComponentDescriptionsFromPodAdditions(podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) ([]v1alpha1.ComponentDescription, error) { + var descriptions []v1alpha1.ComponentDescription + + for _, mainContainer := range podAdditions.Containers { + component, err := GetComponentByName(mainContainer.Name, workspace) + if err != nil { + return nil, err + } + descriptions = append(descriptions, v1alpha1.ComponentDescription{ + Name: component.Name, + PodAdditions: v1alpha1.PodAdditions{ + Containers: []corev1.Container{mainContainer}, + }, + ComponentMetadata: GetComponentMetadata(component.Name, component.Container, workspace.Commands, component.Attributes), + }) + } + + for _, initContainer := range podAdditions.InitContainers { + component, err := GetComponentByName(initContainer.Name, workspace) + if err != nil { + return nil, err + } + descriptions = append(descriptions, v1alpha1.ComponentDescription{ + Name: component.Name, + PodAdditions: v1alpha1.PodAdditions{ + InitContainers: []corev1.Container{initContainer}, + }, + ComponentMetadata: GetComponentMetadata(component.Name, component.Container, workspace.Commands, component.Attributes), + }) + } + + // TODO: It's unclear how ComponentDescriptions accomodates volumes, especially with e.g. the common + // TODO: PVC strategy; no *one* component defines the common PVC, and we don't support multiple components + // TODO: defining the same volume. To work around this, we create a dummy component to store volumes. + descriptions = append(descriptions, v1alpha1.ComponentDescription{ + Name: "devfile-storage-volumes", + PodAdditions: v1alpha1.PodAdditions{ + Volumes: podAdditions.Volumes, + }, + }) + + return descriptions, nil +} + +func GetComponentByName(name string, workspace devworkspace.DevWorkspaceTemplateSpec) (*devworkspace.Component, error) { + for _, component := range workspace.Components { + if name == component.Key() { + return &component, nil + } + } + return nil, fmt.Errorf("component with name %s not found", name) +} + +func GetComponentMetadata(componentName string, container *devworkspace.ContainerComponent, commands []devworkspace.Command, attr attributes.Attributes) v1alpha1.ComponentMetadata { + componentMetadata := v1alpha1.ComponentMetadata{ + Containers: map[string]v1alpha1.ContainerDescription{ + componentName: getComponentContainerDescription(container, attr), + }, + ContributedRuntimeCommands: getDockerfileComponentCommands(componentName, commands), + Endpoints: container.Endpoints, + } + return componentMetadata +} + +func getComponentContainerDescription(container *devworkspace.ContainerComponent, attributes attributes.Attributes) v1alpha1.ContainerDescription { + source := config.RestApisRecipeSourceContainerAttribute + if attributes.Exists("app.kubernetes.io/component") { + source = config.RestApisRecipeSourceToolAttribute + } + + var ports []int + for _, endpoint := range container.Endpoints { + ports = append(ports, endpoint.TargetPort) + } + + return v1alpha1.ContainerDescription{ + Attributes: map[string]string{ + config.RestApisContainerSourceAttribute: source, + }, + Ports: ports, + } +} + +func getDockerfileComponentCommands(componentName string, commands []devworkspace.Command) []v1alpha1.CheWorkspaceCommand { + var componentCommands []v1alpha1.CheWorkspaceCommand + for _, command := range commands { + if command.Exec == nil { + continue + } + if command.Exec.Component == componentName { + attr := map[string]string{ + config.CommandWorkingDirectoryAttribute: command.Exec.WorkingDir, // TODO: Env var substitution? + config.CommandMachineNameAttribute: componentName, + config.ComponentAliasCommandAttribute: componentName, + } + + componentCommands = append(componentCommands, v1alpha1.CheWorkspaceCommand{ + Name: command.Id, + Type: "exec", + CommandLine: command.Exec.CommandLine, + Attributes: attr, + }) + } + } + return componentCommands +} From 6221ca718f167030f63a20872f846c1b4654426d Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Wed, 6 Jan 2021 17:00:39 -0500 Subject: [PATCH 05/14] Make devworkspace reconciler use new devfile 2.0 libraries Replace usage of Component subresource with straight conversion of devfile components to k8s objects. This commit disables support for un-flattened devfiles (i.e. devfiles that contain parents or plugins). For flattened devfiles, functionality is identical. Signed-off-by: Angel Misevski --- .../workspace/devworkspace_controller.go | 52 +++++++++++-------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 4f0042bbc..3f5cd4267 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -19,17 +19,22 @@ import ( "strings" "time" - batchv1 "k8s.io/api/batch/v1" + containerlib "github.com/devfile/devworkspace-operator/pkg/library/container" + shimlib "github.com/devfile/devworkspace-operator/pkg/library/shim" + storagelib "github.com/devfile/devworkspace-operator/pkg/library/storage" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/controllers/workspace/provision" + "github.com/devfile/devworkspace-operator/controllers/workspace/restapis" "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/timing" - appsv1 "k8s.io/api/apps/v1" - k8sErrors "k8s.io/apimachinery/pkg/api/errors" "github.com/go-logr/logr" "github.com/google/uuid" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -37,8 +42,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/controllers/workspace/provision" - "github.com/devfile/devworkspace-operator/controllers/workspace/restapis" ) type currentStatus struct { @@ -167,24 +170,31 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct return reconcile.Result{}, nil } - // Step one: Create components, and wait for their states to be ready. timing.SetTime(workspace, timing.ComponentsCreated) - componentsStatus := provision.SyncComponentsToCluster(workspace, clusterAPI) - if !componentsStatus.Continue { - if componentsStatus.FailStartup { - reqLogger.Info("DevWorkspace start failed") - reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed - if componentsStatus.Message != "" { - reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = componentsStatus.Message - } else { - reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = "Could not find plugins for devworkspace" - } - } else { - reqLogger.Info("Waiting on components to be ready") - } - return reconcile.Result{Requeue: componentsStatus.Requeue}, componentsStatus.Err + // TODO#185 : Move away from using devfile 1.0 constructs; only work on flattened devfiles until + // TODO#185 : plugins is figured out. + // TODO#185 : Implement defaulting container component for Web Terminals for compatibility + devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(workspace.Spec.Template) + if err != nil { + reqLogger.Info("DevWorkspace start failed") + reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed + reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile: %s", err) + return reconcile.Result{}, err + } + err = storagelib.RewriteContainerVolumeMounts(workspace.Status.WorkspaceId, devfilePodAdditions, workspace.Spec.Template) + if err != nil { + reqLogger.Info("DevWorkspace start failed") + reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed + reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile persistent storage: %s", err) + return reconcile.Result{}, err + } + componentDescriptions, err := shimlib.GetComponentDescriptionsFromPodAdditions(devfilePodAdditions, workspace.Spec.Template) + if err != nil { + reqLogger.Info("DevWorkspace start failed") + reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed + reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile for Theia: %s", err) + return reconcile.Result{}, err } - componentDescriptions := componentsStatus.ComponentDescriptions reconcileStatus.Conditions[devworkspace.WorkspaceReady] = "" timing.SetTime(workspace, timing.ComponentsReady) From c8e9579ee77fcb798cc46a1a726646b452e8a2bd Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Wed, 6 Jan 2021 17:47:49 -0500 Subject: [PATCH 06/14] Add flattened devfile samples for testing Signed-off-by: Angel Misevski --- samples/cloud-shell.yaml | 22 --- samples/flattened_theia-next.yaml | 166 ++++++++++++++++++ ...space.yaml => flattened_theia-nodejs.yaml} | 52 +++--- samples/flattened_web-terminal-dev.yaml | 45 +++++ samples/flattened_web-terminal.yaml | 46 +++++ samples/theia-latest.yaml | 25 --- samples/theia-next.yaml | 2 +- samples/theia-nodejs.yaml | 10 +- 8 files changed, 294 insertions(+), 74 deletions(-) delete mode 100644 samples/cloud-shell.yaml create mode 100644 samples/flattened_theia-next.yaml rename samples/{all-in-one-theia-nodejs.devworkspace.yaml => flattened_theia-nodejs.yaml} (84%) create mode 100644 samples/flattened_web-terminal-dev.yaml create mode 100644 samples/flattened_web-terminal.yaml delete mode 100644 samples/theia-latest.yaml diff --git a/samples/cloud-shell.yaml b/samples/cloud-shell.yaml deleted file mode 100644 index a50d43f11..000000000 --- a/samples/cloud-shell.yaml +++ /dev/null @@ -1,22 +0,0 @@ -kind: DevWorkspace -apiVersion: workspace.devfile.io/v1alpha1 -metadata: - name: cloud-shell - annotations: - controller.devfile.io/restricted-access: "true" -spec: - started: true - routingClass: openshift-oauth - template: - components: - - plugin: - name: cloud-shell - id: eclipse/cloud-shell/nightly - - container: - name: dev - image: quay.io/eclipse/che-sidecar-openshift-connector:0.1.2-2601509 - memoryLimit: 256Mi - args: ["tail", "-f", "/dev/null"] - env: - - value: '\[\e[34m\]>\[\e[m\]\[\e[33m\]>\[\e[m\]' - name: PS1 diff --git a/samples/flattened_theia-next.yaml b/samples/flattened_theia-next.yaml new file mode 100644 index 000000000..f2637e6cf --- /dev/null +++ b/samples/flattened_theia-next.yaml @@ -0,0 +1,166 @@ +kind: DevWorkspace +apiVersion: workspace.devfile.io/v1alpha2 +metadata: + name: theia +spec: + started: true + template: + projects: + - name: web-nodejs-sample + git: + remotes: + origin: "https://github.com/che-samples/web-nodejs-sample.git" + components: + ### BEGIN Contributions from machine-exec plugin ### + - name: terminal + attributes: + "app.kubernetes.io/name": che-terminal.eclipse.org + "app.kubernetes.io/part-of": che.eclipse.org + "app.kubernetes.io/component": terminal + container: + image: "quay.io/eclipse/che-machine-exec:nightly" + command: ['/go/bin/che-machine-exec'] + args: + - '--url' + - '0.0.0.0:4444' + - '--pod-selector' + - controller.devfile.io/workspace_id=$(CHE_WORKSPACE_ID) + endpoints: + - name: "che-mach-exec" + exposure: public + targetPort: 4444 + protocol: ws + secure: true + attributes: + type: terminal + cookiesAuthEnabled: "true" + ### END Contributions from machine-exec plugin ### + ### BEGIN Contributions from Theia plugin ### + - name: plugins + volume: {} + - name: remote-endpoint + volume: {} # TODO: Fix this once ephemeral volumes are supported + - name: vsx-installer # Mainly reads the container objects and searches for those + # with che-theia.eclipse.org/vscode-extensions attributes to get VSX urls + # Those found in the dedicated containers components are with a sidecar, + # Those found in the che-theia container are without a sidecar. + attributes: + "app.kubernetes.io/part-of": che-theia.eclipse.org + "app.kubernetes.io/component": bootstrapper + container: + args: + - /bin/sh + - '-c' + - | + KUBE_API_ENDPOINT="https://kubernetes.default.svc/apis/workspace.devfile.io/v1alpha2/namespaces/${CHE_WORKSPACE_NAMESPACE}/devworkspaces/${CHE_WORKSPACE_NAME}" &&\ + TOKEN=$(cat /var/run/secrets/kubernetes.io/serviceaccount/token) &&\ + WORKSPACE=$(curl -fsS --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt -H "Authorization: Bearer ${TOKEN}" $KUBE_API_ENDPOINT) &&\ + for container in $(echo $WORKSPACE | sed -e 's;[[,]\({"attributes":{"app.kubernetes.io\);\n\1;g' | grep '"che-theia.eclipse.org/vscode-extensions":' | grep -e '^{"attributes".*'); do \ + dest=$(echo "$container" | sed 's;.*{"name":"THEIA_PLUGINS","value":"local-dir://\([^"][^"]*\)"}.*;\1;' - ) ;\ + urls=$(echo "$container" | sed 's;.*"che-theia.eclipse.org/vscode-extensions":\[\([^]][^]]*\)\]}.*;\1;' - ) ;\ + mkdir -p $dest ;\ + for url in $(echo $urls | sed 's/[",]/ /g' - ); do \ + echo; echo downloading $urls to $dest; curl -L $url > $dest/$(basename $url) ;\ + done \ + done \ + image: 'quay.io/samsahai/curl:latest' + volumeMounts: + - path: "/plugins" + name: plugins + - name: remote-runtime-injector + attributes: + "app.kubernetes.io/part-of": che-theia.eclipse.org + "app.kubernetes.io/component": bootstrapper + container: #### corresponds to `initContainer` definition in old meta.yaml. + image: "quay.io/eclipse/che-theia-endpoint-runtime-binary:7.20.0" + volumeMounts: + - path: "/remote-endpoint" + name: remote-endpoint + env: + - name: PLUGIN_REMOTE_ENDPOINT_EXECUTABLE + value: /remote-endpoint/plugin-remote-endpoint + - name: REMOTE_ENDPOINT_VOLUME_NAME + value: remote-endpoint + - name: theia-ide + attributes: + "app.kubernetes.io/name": che-theia.eclipse.org + "app.kubernetes.io/part-of": che.eclipse.org + "app.kubernetes.io/component": editor + + # Added by Che-theia at start when detecting, after cloning, that the extensions.json in the repo + # contains the vscode-pull-request-github vscode plugin. + "che-theia.eclipse.org/vscode-extensions": + - https://github.com/microsoft/vscode-pull-request-github/releases/download/v0.8.0/vscode-pull-request-github-0.8.0.vsix + container: + image: "quay.io/eclipse/che-theia:next" + env: + - name: THEIA_PLUGINS + value: local-dir:///plugins + - name: HOSTED_PLUGIN_HOSTNAME + value: 0.0.0.0 + - name: HOSTED_PLUGIN_PORT + value: "3130" + - name: THEIA_HOST + value: 0.0.0.0 + volumeMounts: + - path: "/plugins" + name: plugins + mountSources: true + memoryLimit: "512M" + endpoints: + - name: "theia" + exposure: public + targetPort: 3100 + secure: true + protocol: http + attributes: + type: ide + cookiesAuthEnabled: "true" + - name: "webviews" + exposure: public + targetPort: 3100 + protocol: http + secure: true + attributes: + type: webview + cookiesAuthEnabled: "true" + unique: "true" + - name: "theia-dev" + exposure: public + targetPort: 3130 + protocol: http + attributes: + type: ide-dev + - name: "theia-redir-1" + exposure: public + targetPort: 13131 + protocol: http + - name: "theia-redir-2" + exposure: public + targetPort: 13132 + protocol: http + - name: "theia-redir-3" + exposure: public + targetPort: 13133 + protocol: http + ### END Contributions from Theia plugin ### + + commands: + - id: say-hello + exec: + component: plugin + commandLine: echo "Hello from $(pwd)" + workingDir: ${PROJECTS_ROOT}/project/app + ### BEGIN Contributions from Theia plugin ### + # Commands coming from plugin editor + - id: inject-theia-in-remote-sidecar + apply: + component: remote-runtime-injector + - id: copy-vsx + apply: + component: vsx-installer + events: + preStart: + - inject-theia-in-remote-sidecar + - copy-vsx + ### END Contributions from Theia plugin ### diff --git a/samples/all-in-one-theia-nodejs.devworkspace.yaml b/samples/flattened_theia-nodejs.yaml similarity index 84% rename from samples/all-in-one-theia-nodejs.devworkspace.yaml rename to samples/flattened_theia-nodejs.yaml index 8c48d8321..51a344382 100644 --- a/samples/all-in-one-theia-nodejs.devworkspace.yaml +++ b/samples/flattened_theia-nodejs.yaml @@ -6,7 +6,7 @@ spec: started: true template: projects: - - name: project + - name: web-nodejs-sample git: remotes: origin: "https://github.com/che-samples/web-nodejs-sample.git" @@ -75,11 +75,11 @@ spec: - name: "theia-redir-3" exposure: public targetPort: 13133 - protocol: http + protocol: http - name: plugins - volume: {} - + volume: {} + - name: vsx-installer # Mainly reads the container objects and searches for those # with che-theia.eclipse.org/vscode-extensions attributes to get VSX urls # Those found in the dedicated containers components are with a sidecar, @@ -91,8 +91,18 @@ spec: args: - /bin/sh - '-c' - - > - workspace=$(curl -fsS --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://kubernetes.default.svc/apis/workspace.devfile.io/v1alpha2/namespaces/${CHE_WORKSPACE_NAMESPACE}/devworkspaces/${CHE_WORKSPACE_NAME}) && for container in $(echo $workspace | sed -e 's;[[,]\({"attributes":{"app.kubernetes.io\);\n\1;g' | grep '"che-theia.eclipse.org/vscode-extensions":' | grep -e '^{"attributes".*'); do dest=$(echo "$container" | sed 's;.*{"name":"THEIA_PLUGINS","value":"local-dir://\([^"][^"]*\)"}.*;\1;' - ) ; urls=$(echo "$container" | sed 's;.*"che-theia.eclipse.org/vscode-extensions":\[\([^]][^]]*\)\]}.*;\1;' - ) ; mkdir -p $dest; for url in $(echo $urls | sed 's/[",]/ /g' - ); do echo; echo downloading $urls to $dest; curl -L $url > $dest/$(basename $url); done; done + - | + KUBE_API_ENDPOINT="https://kubernetes.default.svc/apis/workspace.devfile.io/v1alpha2/namespaces/${CHE_WORKSPACE_NAMESPACE}/devworkspaces/${CHE_WORKSPACE_NAME}" &&\ + TOKEN=$(cat /var/run/secrets/kubernetes.io/serviceaccount/token) &&\ + WORKSPACE=$(curl -fsS --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt -H "Authorization: Bearer ${TOKEN}" $KUBE_API_ENDPOINT) &&\ + for container in $(echo $WORKSPACE | sed -e 's;[[,]\({"attributes":{"app.kubernetes.io\);\n\1;g' | grep '"che-theia.eclipse.org/vscode-extensions":' | grep -e '^{"attributes".*'); do \ + dest=$(echo "$container" | sed 's;.*{"name":"THEIA_PLUGINS","value":"local-dir://\([^"][^"]*\)"}.*;\1;' - ) ;\ + urls=$(echo "$container" | sed 's;.*"che-theia.eclipse.org/vscode-extensions":\[\([^]][^]]*\)\]}.*;\1;' - ) ;\ + mkdir -p $dest ;\ + for url in $(echo $urls | sed 's/[",]/ /g' - ); do \ + echo; echo downloading $urls to $dest; curl -L $url > $dest/$(basename $url) ;\ + done \ + done \ image: 'quay.io/samsahai/curl:latest' volumeMounts: - path: "/plugins" @@ -143,7 +153,7 @@ spec: attributes: "app.kubernetes.io/part-of": che-theia.eclipse.org "app.kubernetes.io/component": vscode-plugin - + # Added by Che-theia at start when detecting, after cloning, that the extensions.json in the repo # contains the typescript vscode plugin. "che-theia.eclipse.org/vscode-extensions": @@ -154,7 +164,7 @@ spec: memoryLimit: '512Mi' env: - name: PLUGIN_REMOTE_ENDPOINT_EXECUTABLE - value: /remote-endpoint/plugin-remote-endpoint + value: /remote-endpoint/plugin-remote-endpoint - name: THEIA_PLUGINS value: local-dir:///plugins/sidecars/vscode-typescript volumeMounts: @@ -162,7 +172,7 @@ spec: name: remote-endpoint - name: plugins path: /plugins - + # plugin: # name: node-debug-plugin # id: ms-vscode/node-debug2/latest @@ -171,28 +181,28 @@ spec: attributes: "app.kubernetes.io/part-of": che-theia.eclipse.org "app.kubernetes.io/component": vscode-plugin - + # Added by Che-theia at start when detecting, after cloning, that the extensions.json in the repo # contains the typescript vscode plugin. "che-theia.eclipse.org/vscode-extensions": - https://download.jboss.org/jbosstools/vscode/3rdparty/vscode-node-debug/node-debug-1.41.1.vsix - https://download.jboss.org/jbosstools/vscode/3rdparty/vscode-node-debug2/node-debug2-1.42.3.vsix - + container: image: "quay.io/eclipse/che-sidecar-node:12-026416c" memoryLimit: '512Mi' env: - name: PLUGIN_REMOTE_ENDPOINT_EXECUTABLE - value: /remote-endpoint/plugin-remote-endpoint + value: /remote-endpoint/plugin-remote-endpoint - name: THEIA_PLUGINS value: local-dir:///plugins/sidecars/vscode-node-debug volumeMounts: - path: "/remote-endpoint" name: remote-endpoint - - name: plugins-node-debug-plugin - path: /plugins + - path: "/plugins" + name: plugins - # User runtime container + # User runtime container - name: nodejs container: image: quay.io/eclipse/che-nodejs10-ubi:nightly @@ -214,29 +224,29 @@ spec: component: vsx-installer # User commands - - id: download dependencies + - id: download-dependencies exec: component: nodejs commandLine: npm install workingDir: ${PROJECTS_ROOT}/project/app - - id: run the app + - id: run-the-app exec: component: nodejs commandLine: nodemon app.js workingDir: ${PROJECTS_ROOT}/project/app - - id: run the app (debugging enabled) + - id: run-the-app-(debugging-enabled) exec: component: nodejs commandLine: nodemon --inspect app.js workingDir: ${PROJECTS_ROOT}/project/app - - id: stop the app + - id: stop-the-app exec: component: nodejs commandLine: >- node_server_pids=$(pgrep -fx '.*nodemon (--inspect )?app.js' | tr "\\n" " ") && echo "Stopping node server with PIDs: ${node_server_pids}" && kill -15 ${node_server_pids} &>/dev/null && echo 'Done.' - - id: Attach remote debugger + - id: attach-remote-debugger vscodeLaunch: inlined: | { @@ -253,7 +263,7 @@ spec: } ] } - + events: preStart: - inject-theia-in-remote-sidecar diff --git a/samples/flattened_web-terminal-dev.yaml b/samples/flattened_web-terminal-dev.yaml new file mode 100644 index 000000000..cd2853f11 --- /dev/null +++ b/samples/flattened_web-terminal-dev.yaml @@ -0,0 +1,45 @@ +kind: DevWorkspace +apiVersion: workspace.devfile.io/v1alpha2 +metadata: + name: web-terminal-dev + annotations: + controller.devfile.io/restricted-access: "true" + labels: + # it's a label OpenShift console uses a flag to mark terminal's workspaces + console.openshift.io/terminal: "true" +spec: + started: true + routingClass: 'basic' + template: + components: + # TODO: Remove once defaulting for web terminal is implemented. + - name: dev + container: + image: quay.io/wto/web-terminal-tooling:latest + mountSources: false + memoryLimit: 256Mi + args: ["tail", "-f", "/dev/null"] + env: + - value: '\[\e[34m\]>\[\e[m\]\[\e[33m\]>\[\e[m\]' + name: PS1 + - name: web-terminal + container: + image: quay.io/eclipse/che-machine-exec:nightly + mountSources: false + command: ["/go/bin/che-machine-exec", + "--authenticated-user-id", "$(DEVWORKSPACE_CREATOR)", + "--idle-timeout", "$(DEVWORKSPACE_IDLE_TIMEOUT)", + "--pod-selector", "controller.devfile.io/workspace_id=$(CHE_WORKSPACE_ID)", + "--use-bearer-token"] + endpoints: + - name: web-terminal + targetPort: 4444 + attributes: + protocol: http + type: ide + discoverable: "false" + secure: "true" + cookiesAuthEnabled: "true" + env: + - name: USE_BEARER_TOKEN + value: "true" diff --git a/samples/flattened_web-terminal.yaml b/samples/flattened_web-terminal.yaml new file mode 100644 index 000000000..56d20d488 --- /dev/null +++ b/samples/flattened_web-terminal.yaml @@ -0,0 +1,46 @@ +kind: DevWorkspace +apiVersion: workspace.devfile.io/v1alpha2 +metadata: + name: web-terminal + annotations: + controller.devfile.io/restricted-access: "true" + labels: + # it's a label OpenShift console uses a flag to mark terminal's workspaces + console.openshift.io/terminal: "true" +spec: + started: true + routingClass: 'web-terminal' + template: + components: + # TODO: Remove once defaulting for web terminal is implemented. + - name: dev + container: + image: quay.io/wto/web-terminal-tooling:latest + mountSources: false + memoryLimit: 256Mi + args: ["tail", "-f", "/dev/null"] + env: + - value: '\[\e[34m\]>\[\e[m\]\[\e[33m\]>\[\e[m\]' + name: PS1 + - name: web-terminal + container: + image: quay.io/eclipse/che-machine-exec:nightly + mountSources: false + command: ["/go/bin/che-machine-exec", + "--authenticated-user-id", "$(DEVWORKSPACE_CREATOR)", + "--idle-timeout", "$(DEVWORKSPACE_IDLE_TIMEOUT)", + "--pod-selector", "controller.devfile.io/workspace_id=$(CHE_WORKSPACE_ID)", + "--use-bearer-token", + "--use-tls"] + endpoints: + - name: web-terminal + targetPort: 4444 + attributes: + protocol: http + type: ide + discoverable: "false" + secure: "true" + cookiesAuthEnabled: "true" + env: + - name: USE_BEARER_TOKEN + value: "true" diff --git a/samples/theia-latest.yaml b/samples/theia-latest.yaml deleted file mode 100644 index 805596ef3..000000000 --- a/samples/theia-latest.yaml +++ /dev/null @@ -1,25 +0,0 @@ -kind: DevWorkspace -apiVersion: workspace.devfile.io/v1alpha2 -metadata: - name: theia -spec: - started: true - template: - projects: - - name: project - git: - remotes: - origin: "https://github.com/che-samples/web-nodejs-sample.git" - components: - - name: theia - plugin: - id: eclipse/che-theia/latest - - name: terminal - plugin: - id: eclipse/che-machine-exec-plugin/latest - commands: - - id: say hello - exec: - component: plugin - commandLine: echo "Hello from $(pwd)" - workingDir: ${PROJECTS_ROOT}/project/app diff --git a/samples/theia-next.yaml b/samples/theia-next.yaml index 53a940fdd..cd19e8d6b 100644 --- a/samples/theia-next.yaml +++ b/samples/theia-next.yaml @@ -18,7 +18,7 @@ spec: plugin: id: eclipse/che-machine-exec-plugin/nightly commands: - - id: say hello + - id: say-hello exec: component: plugin commandLine: echo "Hello from $(pwd)" diff --git a/samples/theia-nodejs.yaml b/samples/theia-nodejs.yaml index 4dcd0c512..f81b9ae54 100644 --- a/samples/theia-nodejs.yaml +++ b/samples/theia-nodejs.yaml @@ -33,29 +33,29 @@ spec: mountSources: true commands: - exec: - id: download dependencies + id: download-dependencies component: nodejs commandLine: npm install workingDir: ${PROJECTS_ROOT}/project/app - exec: - id: run the app + id: run-the-app component: nodejs commandLine: nodemon app.js workingDir: ${PROJECTS_ROOT}/project/app - exec: - id: run the app (debugging enabled) + id: run-the-app-(debugging-enabled) component: nodejs commandLine: nodemon --inspect app.js workingDir: ${PROJECTS_ROOT}/project/app - exec: - id: stop the app + id: stop-the-app component: nodejs commandLine: >- node_server_pids=$(pgrep -fx '.*nodemon (--inspect )?app.js' | tr "\\n" " ") && echo "Stopping node server with PIDs: ${node_server_pids}" && kill -15 ${node_server_pids} &>/dev/null && echo 'Done.' - vscodeLaunch: - id: Attach remote debugger + id: Attach-remote-debugger inlined: | { "version": "0.2.0", From 680e04b4d68d7f00eb64c742b3fb0d0fd78a2a63 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 7 Jan 2021 12:33:44 -0500 Subject: [PATCH 07/14] Improve volume handling in library to correctly manage mountSources - Add documentation to various methods as library functions deal with more edge cases - Split package pkg/library/container into multiple files for readability - Improve handling of mountSources to respect sourceMapping, correctly define projects volume - Improve common PVC provisioning to not add PVC if not needed Signed-off-by: Angel Misevski --- pkg/library/constants/constants.go | 20 ++++ pkg/library/container/container.go | 145 +++----------------------- pkg/library/container/conversion.go | 105 +++++++++++++++++++ pkg/library/container/mountSources.go | 60 +++++++++++ pkg/library/storage/commonStorage.go | 82 ++++++++++++--- 5 files changed, 264 insertions(+), 148 deletions(-) create mode 100644 pkg/library/constants/constants.go create mode 100644 pkg/library/container/conversion.go create mode 100644 pkg/library/container/mountSources.go diff --git a/pkg/library/constants/constants.go b/pkg/library/constants/constants.go new file mode 100644 index 000000000..9531943a5 --- /dev/null +++ b/pkg/library/constants/constants.go @@ -0,0 +1,20 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +// Package constants contains constants related to the devfile API spec (e.g. reserved env vars) +package constants + +const ( + ProjectsRootEnvVar = "PROJECTS_ROOT" + ProjectsSourceEnvVar = "PROJECTS_SOURCE" + ProjectsVolumeName = "projects" +) diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go index 7d89db7f3..1d5b0ea7f 100644 --- a/pkg/library/container/container.go +++ b/pkg/library/container/container.go @@ -12,6 +12,13 @@ // Package container contains library functions for converting DevWorkspace Container components to Kubernetes // components +// +// TODO: +// - Devfile API spec is unclear on how mountSources should be handled -- mountPath is assumed to be /projects +// and volume name is assumed to be "projects" +// see issues: +// - https://github.com/devfile/api/issues/290 +// - https://github.com/devfile/api/issues/291 package container import ( @@ -19,23 +26,19 @@ import ( devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/library" "github.com/devfile/devworkspace-operator/pkg/library/lifecycle" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) - -const ( - ProjectsRootEnvVar = "PROJECTS_ROOT" - ProjectsSourceEnvVar = "PROJECTS_SOURCE" - ProjecstVolumeName = "projects" ) // GetKubeContainersFromDevfile converts container components in a DevWorkspace into Kubernetes containers. // If a DevWorkspace container is an init container (i.e. is bound to a preStart event), it will be returned as an // init container. // +// This function also provisions volume mounts on containers as follows: +// - Container component's volume mounts are provisioned with the mount path and name specified in the devworkspace +// However, no Volumes are added to the returned PodAdditions at this stage; the volumeMounts above are expected to be +// rewritten as Volumes are added to PodAdditions, in order to support e.g. using one PVC to hold all volumes +// // Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin) func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpec) (*v1alpha1.PodAdditions, error) { if !library.DevWorkspaceIsFlattened(workspace) { @@ -56,6 +59,7 @@ func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpe if err != nil { return nil, err } + handleMountSources(k8sContainer, component.Container) podAdditions.Containers = append(podAdditions.Containers, *k8sContainer) } @@ -64,130 +68,9 @@ func GetKubeContainersFromDevfile(workspace devworkspace.DevWorkspaceTemplateSpe if err != nil { return nil, err } + handleMountSources(k8sContainer, container.Container) podAdditions.InitContainers = append(podAdditions.InitContainers, *k8sContainer) } - fillDefaultEnvVars(podAdditions, workspace) - return podAdditions, nil } - -func convertContainerToK8s(devfileComponent devworkspace.Component) (*corev1.Container, error) { - if devfileComponent.Container == nil { - return nil, fmt.Errorf("cannot get k8s container from non-container component") - } - devfileContainer := devfileComponent.Container - - containerResources, err := devfileResourcesToContainerResources(devfileContainer) - if err != nil { - return nil, err - } - - var mountSources bool - if devfileContainer.MountSources == nil { - mountSources = true - } else { - mountSources = *devfileContainer.MountSources - } - - container := &corev1.Container{ - Name: devfileComponent.Name, - Image: devfileContainer.Image, - Command: devfileContainer.Command, - Args: devfileContainer.Args, - Resources: *containerResources, - Ports: devfileEndpointsToContainerPorts(devfileContainer.Endpoints), - Env: devfileEnvToContainerEnv(devfileContainer.Env), - VolumeMounts: devfileVolumeMountsToContainerVolumeMounts(devfileContainer.VolumeMounts, mountSources), - ImagePullPolicy: corev1.PullPolicy(config.ControllerCfg.GetSidecarPullPolicy()), - } - - return container, nil -} - -func devfileEndpointsToContainerPorts(endpoints []devworkspace.Endpoint) []corev1.ContainerPort { - var containerPorts []corev1.ContainerPort - for _, endpoint := range endpoints { - containerPorts = append(containerPorts, corev1.ContainerPort{ - Name: endpoint.Name, - ContainerPort: int32(endpoint.TargetPort), - Protocol: corev1.ProtocolTCP, - }) - } - return containerPorts -} - -func devfileResourcesToContainerResources(devfileContainer *devworkspace.ContainerComponent) (*corev1.ResourceRequirements, error) { - // TODO: Handle memory request and CPU when implemented in devfile API - memLimit := devfileContainer.MemoryLimit - if memLimit == "" { - memLimit = config.SidecarDefaultMemoryLimit - } - memLimitQuantity, err := resource.ParseQuantity(memLimit) - if err != nil { - return nil, fmt.Errorf("failed to parse memory limit %q: %w", memLimit, err) - } - return &corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceMemory: memLimitQuantity, - }, - }, nil -} - -func devfileVolumeMountsToContainerVolumeMounts(devfileVolumeMounts []devworkspace.VolumeMount, mountSources bool) []corev1.VolumeMount { - var volumeMounts []corev1.VolumeMount - for _, vm := range devfileVolumeMounts { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: vm.Name, - MountPath: vm.Path, - }) - } - if mountSources { - volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: ProjecstVolumeName, - MountPath: config.DefaultProjectsSourcesRoot, - }) - } - return volumeMounts -} - -func devfileEnvToContainerEnv(devfileEnvVars []devworkspace.EnvVar) []corev1.EnvVar { - var env []corev1.EnvVar - for _, devfileEnv := range devfileEnvVars { - env = append(env, corev1.EnvVar{ - Name: devfileEnv.Name, - Value: devfileEnv.Value, - }) - } - return env -} - -func fillDefaultEnvVars(podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) { - var projectsSource string - if len(workspace.Projects) > 0 { - // TODO: Unclear from devfile spec how this should work when there are multiple projects - projectsSource = fmt.Sprintf("%s/%s", config.DefaultProjectsSourcesRoot, workspace.Projects[0].ClonePath) - } else { - projectsSource = config.DefaultProjectsSourcesRoot - } - - // Add devfile reserved env var and legacy env var for Che-Theia - for idx, container := range podAdditions.Containers { - podAdditions.Containers[idx].Env = append(container.Env, corev1.EnvVar{ - Name: ProjectsRootEnvVar, - Value: config.DefaultProjectsSourcesRoot, - }, corev1.EnvVar{ - Name: ProjectsSourceEnvVar, - Value: projectsSource, - }) - } - for idx, container := range podAdditions.InitContainers { - podAdditions.InitContainers[idx].Env = append(container.Env, corev1.EnvVar{ - Name: ProjectsRootEnvVar, - Value: config.DefaultProjectsSourcesRoot, - }, corev1.EnvVar{ - Name: ProjectsSourceEnvVar, - Value: projectsSource, - }) - } -} diff --git a/pkg/library/container/conversion.go b/pkg/library/container/conversion.go new file mode 100644 index 000000000..06d5d3d22 --- /dev/null +++ b/pkg/library/container/conversion.go @@ -0,0 +1,105 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package container + +import ( + "fmt" + + "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/config" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func convertContainerToK8s(devfileComponent v1alpha2.Component) (*v1.Container, error) { + if devfileComponent.Container == nil { + return nil, fmt.Errorf("cannot get k8s container from non-container component") + } + devfileContainer := devfileComponent.Container + + containerResources, err := devfileResourcesToContainerResources(devfileContainer) + if err != nil { + return nil, err + } + + container := &v1.Container{ + Name: devfileComponent.Name, + Image: devfileContainer.Image, + Command: devfileContainer.Command, + Args: devfileContainer.Args, + Resources: *containerResources, + Ports: devfileEndpointsToContainerPorts(devfileContainer.Endpoints), + Env: devfileEnvToContainerEnv(devfileContainer.Env), + VolumeMounts: devfileVolumeMountsToContainerVolumeMounts(devfileContainer.VolumeMounts), + ImagePullPolicy: v1.PullPolicy(config.ControllerCfg.GetSidecarPullPolicy()), + } + + return container, nil +} + +func devfileEndpointsToContainerPorts(endpoints []v1alpha2.Endpoint) []v1.ContainerPort { + var containerPorts []v1.ContainerPort + for _, endpoint := range endpoints { + containerPorts = append(containerPorts, v1.ContainerPort{ + // Use meaningless name for port since endpoint.Name does not match requirements for ContainerPort name + Name: fmt.Sprintf("%d-%s", endpoint.TargetPort, endpoint.Protocol), + ContainerPort: int32(endpoint.TargetPort), + Protocol: v1.ProtocolTCP, + }) + } + return containerPorts +} + +func devfileResourcesToContainerResources(devfileContainer *v1alpha2.ContainerComponent) (*v1.ResourceRequirements, error) { + // TODO: Handle memory request and CPU when implemented in devfile API + memLimit := devfileContainer.MemoryLimit + if memLimit == "" { + memLimit = config.SidecarDefaultMemoryLimit + } + memLimitQuantity, err := resource.ParseQuantity(memLimit) + if err != nil { + return nil, fmt.Errorf("failed to parse memory limit %q: %w", memLimit, err) + } + return &v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: memLimitQuantity, + }, + }, nil +} + +func devfileVolumeMountsToContainerVolumeMounts(devfileVolumeMounts []v1alpha2.VolumeMount) []v1.VolumeMount { + var volumeMounts []v1.VolumeMount + for _, vm := range devfileVolumeMounts { + path := vm.Path + if path == "" { + // Devfile API spec: if path is unspecified, default is to use volume name + path = fmt.Sprintf("/%s", vm.Name) + } + volumeMounts = append(volumeMounts, v1.VolumeMount{ + Name: vm.Name, + MountPath: path, + }) + } + return volumeMounts +} + +func devfileEnvToContainerEnv(devfileEnvVars []v1alpha2.EnvVar) []v1.EnvVar { + var env []v1.EnvVar + for _, devfileEnv := range devfileEnvVars { + env = append(env, v1.EnvVar{ + Name: devfileEnv.Name, + Value: devfileEnv.Value, + }) + } + return env +} diff --git a/pkg/library/container/mountSources.go b/pkg/library/container/mountSources.go new file mode 100644 index 000000000..0c93b021a --- /dev/null +++ b/pkg/library/container/mountSources.go @@ -0,0 +1,60 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package container + +import ( + devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/library/constants" + corev1 "k8s.io/api/core/v1" +) + +// HasMountSources evaluates whether project sources should be mounted in the given container component. +// MountSources is by default true for non-plugin components, unless they have dedicatedPod set +// TODO: +// - Support dedicatedPod field +// - Find way to track is container component comes from plugin +func HasMountSources(devfileContainer *devworkspace.ContainerComponent) bool { + var mountSources bool + if devfileContainer.MountSources == nil { + mountSources = true + } else { + mountSources = *devfileContainer.MountSources + } + return mountSources +} + +// handleMountSources adds a volumeMount to a container if the corresponding devfile container has +// mountSources enabled. +func handleMountSources(k8sContainer *corev1.Container, devfileContainer *devworkspace.ContainerComponent) { + if !HasMountSources(devfileContainer) { + return + } + sourceMapping := devfileContainer.SourceMapping + if sourceMapping == "" { + // Sanity check -- this value should be defaulted to `/projects` but may not be + // if struct was not processed by k8s + sourceMapping = config.DefaultProjectsSourcesRoot + } + k8sContainer.VolumeMounts = append(k8sContainer.VolumeMounts, corev1.VolumeMount{ + Name: constants.ProjectsVolumeName, + MountPath: sourceMapping, + }) + k8sContainer.Env = append(k8sContainer.Env, corev1.EnvVar{ + Name: constants.ProjectsRootEnvVar, + Value: sourceMapping, + }, corev1.EnvVar{ + Name: constants.ProjectsSourceEnvVar, + Value: sourceMapping, // TODO: Unclear how this should be handled in case of multiple projects + }) +} diff --git a/pkg/library/storage/commonStorage.go b/pkg/library/storage/commonStorage.go index 5fdb7da7f..df14f92f3 100644 --- a/pkg/library/storage/commonStorage.go +++ b/pkg/library/storage/commonStorage.go @@ -10,11 +10,23 @@ // Red Hat, Inc. - initial API and implementation // +// Package storage contains library functions for provisioning volumes and volumeMounts in containers according to the +// volume components in a devfile. These functions also handle mounting project sources to containers that require it. +// +// TODO: +// - Add functionality for generating PVCs with the appropriate size based on size requests in the devfile +// - Devfile API spec is unclear on how mountSources should be handled -- mountPath is assumed to be /projects +// and volume name is assumed to be "projects" +// see issues: +// - https://github.com/devfile/api/issues/290 +// - https://github.com/devfile/api/issues/291 package storage import ( "fmt" + "github.com/devfile/devworkspace-operator/pkg/library/constants" + containerlib "github.com/devfile/devworkspace-operator/pkg/library/container" corev1 "k8s.io/api/core/v1" devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" @@ -22,37 +34,52 @@ import ( "github.com/devfile/devworkspace-operator/pkg/config" ) -func RewriteContainerVolumeMounts(workspaceId string, podAdditions *v1alpha1.PodAdditions, devfile devworkspace.DevWorkspaceTemplateSpec) error { +// RewriteContainerVolumeMounts rewrites the VolumeMounts in a set of PodAdditions according to the 'common' PVC strategy +// (i.e. all volume mounts are subpaths into a common PVC used by all workspaces in the namespace). +// +// Also adds appropriate k8s Volumes to PodAdditions to accomodate the rewritten VolumeMounts. +func RewriteContainerVolumeMounts(workspaceId string, podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) error { + if !NeedsStorage(workspace) { + return nil + } devfileVolumes := map[string]devworkspace.VolumeComponent{} - for _, component := range devfile.Components { + for _, component := range workspace.Components { if component.Volume != nil { if _, exists := devfileVolumes[component.Name]; exists { - return fmt.Errorf("volume component %s is defined multiple times", component.Name) + return fmt.Errorf("volume component '%s' is defined multiple times", component.Name) } devfileVolumes[component.Name] = *component.Volume } } + if _, exists := devfileVolumes[constants.ProjectsVolumeName]; !exists { + // Add implicit projects volume to support mountSources + projectsVolume := devworkspace.VolumeComponent{} + projectsVolume.Size = config.PVCStorageSize + devfileVolumes[constants.ProjectsVolumeName] = projectsVolume + } + // TODO: Support more than the common PVC strategy here (storage provisioner interface?) // TODO: What should we do when a volume isn't explicitly defined? commonPVCName := config.ControllerCfg.GetWorkspacePVCName() - for cIdx, container := range podAdditions.Containers { - for vmIdx, vm := range container.VolumeMounts { - if _, ok := devfileVolumes[vm.Name]; !ok { - return fmt.Errorf("container %s references undefined volume %s", container.Name, vm.Name) + rewriteVolumeMounts := func(containers []corev1.Container) error { + for cIdx, container := range containers { + for vmIdx, vm := range container.VolumeMounts { + if _, ok := devfileVolumes[vm.Name]; !ok { + return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name) + } + containers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name) + containers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName } - podAdditions.Containers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name) - podAdditions.Containers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName } + return nil } - for cIdx, container := range podAdditions.InitContainers { - for vmIdx, vm := range container.VolumeMounts { - if _, ok := devfileVolumes[vm.Name]; !ok { - return fmt.Errorf("container %s references undefined volume %s", container.Name, vm.Name) - } - podAdditions.InitContainers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name) - podAdditions.InitContainers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName - } + if err := rewriteVolumeMounts(podAdditions.Containers); err != nil { + return err } + if err := rewriteVolumeMounts(podAdditions.InitContainers); err != nil { + return err + } + podAdditions.Volumes = append(podAdditions.Volumes, corev1.Volume{ Name: commonPVCName, VolumeSource: corev1.VolumeSource{ @@ -63,3 +90,24 @@ func RewriteContainerVolumeMounts(workspaceId string, podAdditions *v1alpha1.Pod }) return nil } + +// NeedsStorage returns true if storage will need to be provisioned for the current workspace +// TODO: +// - This function is used to decide if we need to create a PVC; need to figure out how to handle +// case of ephemeral storage only +func NeedsStorage(workspace devworkspace.DevWorkspaceTemplateSpec) bool { + for _, component := range workspace.Components { + if component.Volume != nil { + return true + } + if component.Container != nil { + if len(component.Container.VolumeMounts) > 0 { + return true + } + if containerlib.HasMountSources(component.Container) { + return true + } + } + } + return false +} From cdec8d5f3bc703bc51da7dad2d3712322dd9ad87 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 7 Jan 2021 12:40:30 -0500 Subject: [PATCH 08/14] Update reconciler to use library to decide if storage is needed - Only set storage finalizer on workspace if workspace needs storage as defined by storage library - Only generate and sync PVC if workspace needs storage Signed-off-by: Angel Misevski --- .../workspace/devworkspace_controller.go | 18 ++++++----- controllers/workspace/finalize.go | 8 ++--- controllers/workspace/provision/deployment.go | 20 ++----------- controllers/workspace/provision/pvc.go | 30 +------------------ 4 files changed, 17 insertions(+), 59 deletions(-) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 3f5cd4267..4b624ad92 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -179,21 +179,21 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct reqLogger.Info("DevWorkspace start failed") reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile: %s", err) - return reconcile.Result{}, err + return reconcile.Result{}, nil } err = storagelib.RewriteContainerVolumeMounts(workspace.Status.WorkspaceId, devfilePodAdditions, workspace.Spec.Template) if err != nil { reqLogger.Info("DevWorkspace start failed") reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed - reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile persistent storage: %s", err) - return reconcile.Result{}, err + reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile volumes: %s", err) + return reconcile.Result{}, nil } componentDescriptions, err := shimlib.GetComponentDescriptionsFromPodAdditions(devfilePodAdditions, workspace.Spec.Template) if err != nil { reqLogger.Info("DevWorkspace start failed") reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile for Theia: %s", err) - return reconcile.Result{}, err + return reconcile.Result{}, nil } reconcileStatus.Conditions[devworkspace.WorkspaceReady] = "" timing.SetTime(workspace, timing.ComponentsReady) @@ -211,9 +211,11 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct componentDescriptions = append([]controllerv1alpha1.ComponentDescription{cheRestApisComponent}, componentDescriptions...) } - pvcStatus := provision.SyncPVC(workspace, componentDescriptions, r.Client, reqLogger) - if pvcStatus.Err != nil || !pvcStatus.Continue { - return reconcile.Result{Requeue: true}, pvcStatus.Err + if storagelib.NeedsStorage(workspace.Spec.Template) { + pvcStatus := provision.SyncPVC(workspace, r.Client, reqLogger) + if pvcStatus.Err != nil || !pvcStatus.Continue { + return reconcile.Result{Requeue: true}, pvcStatus.Err + } } rbacStatus := provision.SyncRBAC(workspace, r.Client, reqLogger) @@ -283,7 +285,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct // Step six: Create deployment and wait for it to be ready timing.SetTime(workspace, timing.DeploymentCreated) - deploymentStatus := provision.SyncDeploymentToCluster(workspace, podAdditions, componentDescriptions, serviceAcctName, clusterAPI) + deploymentStatus := provision.SyncDeploymentToCluster(workspace, podAdditions, serviceAcctName, clusterAPI) if !deploymentStatus.Continue { if deploymentStatus.FailStartup { reqLogger.Info("Workspace start failed") diff --git a/controllers/workspace/finalize.go b/controllers/workspace/finalize.go index 8e3778b23..f2861b392 100644 --- a/controllers/workspace/finalize.go +++ b/controllers/workspace/finalize.go @@ -18,6 +18,8 @@ import ( "path" "time" + storagelib "github.com/devfile/devworkspace-operator/pkg/library/storage" + "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/controllers/workspace/provision" "github.com/devfile/devworkspace-operator/internal/images" @@ -223,10 +225,8 @@ func (r *DevWorkspaceReconciler) getClusterCleanupJob(ctx context.Context, works return clusterJob, nil } -func isFinalizerNecessary(_ *v1alpha2.DevWorkspace) bool { - // TODO: Implement checking whether persistent storage is used (once other choices are possible) - // Note this could interfere with cloud-shell until this TODO is resolved. - return true +func isFinalizerNecessary(workspace *v1alpha2.DevWorkspace) bool { + return storagelib.NeedsStorage(workspace.Spec.Template) } func hasFinalizer(workspace *v1alpha2.DevWorkspace) bool { diff --git a/controllers/workspace/provision/deployment.go b/controllers/workspace/provision/deployment.go index d0a224eb5..9f079551f 100644 --- a/controllers/workspace/provision/deployment.go +++ b/controllers/workspace/provision/deployment.go @@ -59,13 +59,12 @@ var deploymentDiffOpts = cmp.Options{ func SyncDeploymentToCluster( workspace *devworkspace.DevWorkspace, podAdditions []v1alpha1.PodAdditions, - components []v1alpha1.ComponentDescription, saName string, clusterAPI ClusterAPI) DeploymentProvisioningStatus { // [design] we have to pass components and routing pod additions separately because we need mountsources from each // component. - specDeployment, err := getSpecDeployment(workspace, podAdditions, components, saName, clusterAPI.Scheme) + specDeployment, err := getSpecDeployment(workspace, podAdditions, saName, clusterAPI.Scheme) if err != nil { return DeploymentProvisioningStatus{ ProvisioningStatus: ProvisioningStatus{ @@ -160,7 +159,6 @@ func checkDeploymentStatus(deployment *appsv1.Deployment) (ready bool) { func getSpecDeployment( workspace *devworkspace.DevWorkspace, podAdditionsList []v1alpha1.PodAdditions, - components []v1alpha1.ComponentDescription, saName string, scheme *runtime.Scheme) (*appsv1.Deployment, error) { replicas := int32(1) @@ -225,8 +223,7 @@ func getSpecDeployment( }, } - if IsPVCRequired(components) { - deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, getPersistentVolumeClaim()) + if len(podAdditions.Volumes) > 0 { // Kubernetes creates directories in a PVC to support subpaths such that only the leaf directory has g+rwx permissions. // This means that mounting the subpath e.g. /plugins will result in the directory being // created with 755 permissions, requiring the root UID to remove it. @@ -345,19 +342,6 @@ func mergePodAdditions(toMerge []v1alpha1.PodAdditions) (*v1alpha1.PodAdditions, return podAdditions, nil } -func getPersistentVolumeClaim() corev1.Volume { - var workspaceClaim = corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: config.ControllerCfg.GetWorkspacePVCName(), - } - pvcVolume := corev1.Volume{ - Name: config.ControllerCfg.GetWorkspacePVCName(), - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &workspaceClaim, - }, - } - return pvcVolume -} - func getWorkspaceSubpathVolumeMount(workspaceId string) corev1.VolumeMount { volumeName := config.ControllerCfg.GetWorkspacePVCName() diff --git a/controllers/workspace/provision/pvc.go b/controllers/workspace/provision/pvc.go index 5b5ae9414..316226015 100644 --- a/controllers/workspace/provision/pvc.go +++ b/controllers/workspace/provision/pvc.go @@ -14,7 +14,6 @@ package provision import ( devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/pkg/config" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -23,11 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func SyncPVC(workspace *devworkspace.DevWorkspace, components []v1alpha1.ComponentDescription, client client.Client, reqLogger logr.Logger) ProvisioningStatus { - if !IsPVCRequired(components) { - return ProvisioningStatus{Continue: true} - } - +func SyncPVC(workspace *devworkspace.DevWorkspace, client client.Client, reqLogger logr.Logger) ProvisioningStatus { pvc, err := generatePVC(workspace) if err != nil { return ProvisioningStatus{Err: err} @@ -61,26 +56,3 @@ func generatePVC(workspace *devworkspace.DevWorkspace) (*corev1.PersistentVolume }, }, nil } - -// IsPVCRequired checks to see if we need a PVC for the given devfile. -// If there is any Containers with VolumeMounts that have the same name as the workspace PVC name then we need a PVC -func IsPVCRequired(components []v1alpha1.ComponentDescription) bool { - volumeName := config.ControllerCfg.GetWorkspacePVCName() - for _, comp := range components { - for _, cont := range comp.PodAdditions.Containers { - for _, vm := range cont.VolumeMounts { - if vm.Name == volumeName { - return true - } - } - } - for _, cont := range comp.PodAdditions.InitContainers { - for _, vm := range cont.VolumeMounts { - if vm.Name == volumeName { - return true - } - } - } - } - return false -} From c5c0fd9eee8ec2b382f9a662ea2f6a7456eff680 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 7 Jan 2021 16:22:23 -0500 Subject: [PATCH 09/14] Add tests for storage and container libraries Signed-off-by: Angel Misevski --- pkg/config/config.go | 4 + pkg/library/container/container_test.go | 93 ++++++++++ .../testdata/converts-all-fields.yaml | 74 ++++++++ .../testdata/detects-init-containers.yaml | 37 ++++ .../container/testdata/error-has-parent.yaml | 12 ++ .../container/testdata/error-has-plugins.yaml | 13 ++ .../testdata/error-invalid-resources.yaml | 11 ++ .../testdata/handles-mountSources.yaml | 60 ++++++ .../container/testdata/handles-resources.yaml | 30 +++ .../ignores-non-container-components.yaml | 20 ++ pkg/library/lifecycle/lifecycle_test.go | 3 + pkg/library/storage/commonStorage_test.go | 172 ++++++++++++++++++ .../does-nothing-for-no-storage-needed.yaml | 22 +++ .../testdata/error-duplicate-volumes.yaml | 27 +++ ...error-undefined-volume-init-container.yaml | 24 +++ .../testdata/error-undefined-volume.yaml | 24 +++ .../testdata/projects-volume-overriding.yaml | 35 ++++ ...rites-volumes-for-common-pvc-strategy.yaml | 59 ++++++ 18 files changed, 720 insertions(+) create mode 100644 pkg/library/container/container_test.go create mode 100644 pkg/library/container/testdata/converts-all-fields.yaml create mode 100644 pkg/library/container/testdata/detects-init-containers.yaml create mode 100644 pkg/library/container/testdata/error-has-parent.yaml create mode 100644 pkg/library/container/testdata/error-has-plugins.yaml create mode 100644 pkg/library/container/testdata/error-invalid-resources.yaml create mode 100644 pkg/library/container/testdata/handles-mountSources.yaml create mode 100644 pkg/library/container/testdata/handles-resources.yaml create mode 100644 pkg/library/container/testdata/ignores-non-container-components.yaml create mode 100644 pkg/library/storage/commonStorage_test.go create mode 100644 pkg/library/storage/testdata/does-nothing-for-no-storage-needed.yaml create mode 100644 pkg/library/storage/testdata/error-duplicate-volumes.yaml create mode 100644 pkg/library/storage/testdata/error-undefined-volume-init-container.yaml create mode 100644 pkg/library/storage/testdata/error-undefined-volume.yaml create mode 100644 pkg/library/storage/testdata/projects-volume-overriding.yaml create mode 100644 pkg/library/storage/testdata/rewrites-volumes-for-common-pvc-strategy.yaml diff --git a/pkg/config/config.go b/pkg/config/config.go index 754d2c0d7..e4a9959a9 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -251,6 +251,10 @@ func WatchControllerConfig(mgr manager.Manager) error { return err } +func SetupConfigForTesting(cm *corev1.ConfigMap) { + ControllerCfg.update(cm) +} + func buildDefaultConfigMap(cm *corev1.ConfigMap) { cm.Name = ConfigMapReference.Name cm.Namespace = ConfigMapReference.Namespace diff --git a/pkg/library/container/container_test.go b/pkg/library/container/container_test.go new file mode 100644 index 000000000..77feff4f5 --- /dev/null +++ b/pkg/library/container/container_test.go @@ -0,0 +1,93 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package container + +import ( + "fmt" + "io/ioutil" + "path/filepath" + "testing" + + corev1 "k8s.io/api/core/v1" + + devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" +) + +type testCase struct { + Name string `json:"name,omitempty"` + Input devworkspace.DevWorkspaceTemplateSpec `json:"input,omitempty"` + Output testOutput `json:"output,omitempty"` +} + +type testOutput struct { + PodAdditions *v1alpha1.PodAdditions `json:"podAdditions,omitempty"` + ErrRegexp *string `json:"errRegexp,omitempty"` +} + +var testControllerCfg = &corev1.ConfigMap{ + Data: map[string]string{ + "devworkspace.sidecar.image_pull_policy": "Always", + }, +} + +func setupControllerCfg() { + config.SetupConfigForTesting(testControllerCfg) +} + +func loadTestCaseOrPanic(t *testing.T, testFilename string) testCase { + testPath := filepath.Join("./testdata", testFilename) + bytes, err := ioutil.ReadFile(testPath) + if err != nil { + t.Fatal(err) + } + var test testCase + if err := yaml.Unmarshal(bytes, &test); err != nil { + t.Fatal(err) + } + t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) + return test +} + +func TestGetKubeContainersFromDevfile(t *testing.T) { + tests := []testCase{ + loadTestCaseOrPanic(t, "detects-init-containers.yaml"), + loadTestCaseOrPanic(t, "handles-mountSources.yaml"), + loadTestCaseOrPanic(t, "handles-resources.yaml"), + loadTestCaseOrPanic(t, "ignores-non-container-components.yaml"), + loadTestCaseOrPanic(t, "converts-all-fields.yaml"), + loadTestCaseOrPanic(t, "error-has-parent.yaml"), + loadTestCaseOrPanic(t, "error-has-plugins.yaml"), + loadTestCaseOrPanic(t, "error-invalid-resources.yaml"), + } + setupControllerCfg() + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + // sanity check that file is read correctly. + assert.True(t, len(tt.Input.Components) > 0, "Input defines no components") + gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input) + if tt.Output.ErrRegexp != nil && assert.Error(t, err) { + assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") + } else { + if !assert.NoError(t, err, "Should not return error") { + return + } + assert.Equal(t, tt.Output.PodAdditions, gotPodAdditions, "PodAdditions should match expected output") + } + }) + } +} diff --git a/pkg/library/container/testdata/converts-all-fields.yaml b/pkg/library/container/testdata/converts-all-fields.yaml new file mode 100644 index 000000000..f528af194 --- /dev/null +++ b/pkg/library/container/testdata/converts-all-fields.yaml @@ -0,0 +1,74 @@ +name: "Converts all applicable fields" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + memoryLimit: 999Mi # isolate test to not include memoryLimit + sourceMapping: "/source-mapping" + command: ["test", "command"] + args: ["test", "args"] + env: + - name: "TEST_ENVVAR" + value: "TEST_VALUE" + endpoints: + - name: "test-endpoint-1" + exposure: public + targetPort: 3100 + secure: true + protocol: wss + attributes: + type: ide + cookiesAuthEnabled: "true" + - name: "test-endpoint-2" + exposure: public + targetPort: 8080 + secure: true + protocol: http + attributes: + cookiesAuthEnabled: "true" + volumeMounts: + - name: "test-volume1" + path: "/test-volume1-path" + - name: "test-volume2" + # path omitted; should use name as mountpath + - name: "should-be-ignored" + volume: {} + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + env: + - name: "TEST_ENVVAR" + value: "TEST_VALUE" + - name: "PROJECTS_ROOT" + value: "/source-mapping" + - name: "PROJECTS_SOURCE" + value: "/source-mapping" # Temp value until projects is figured out + command: + - "test" + - "command" + args: + - "test" + - "args" + ports: + - name: "3100-wss" + containerPort: 3100 + protocol: TCP + - name: "8080-http" + containerPort: 8080 + protocol: TCP + volumeMounts: + - name: "test-volume1" + mountPath: "/test-volume1-path" + - name: "test-volume2" + mountPath: "/test-volume2" + - name: "projects" + mountPath: "/source-mapping" diff --git a/pkg/library/container/testdata/detects-init-containers.yaml b/pkg/library/container/testdata/detects-init-containers.yaml new file mode 100644 index 000000000..c997aca8a --- /dev/null +++ b/pkg/library/container/testdata/detects-init-containers.yaml @@ -0,0 +1,37 @@ +name: "Gets initContainers correctly" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + mountSources: false # isolate test to not include volumes + memoryLimit: "999Mi" # isolate test to not include memoryLimit + - name: testing-container-2 + container: + image: testing-image-2 + mountSources: false # isolate test to not include volumes + memoryLimit: "999Mi" # isolate test to not include memoryLimit + commands: + - id: test_preStart_command + apply: + component: testing-container-2 + events: + preStart: + - "test_preStart_command" +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + initContainers: + - name: testing-container-2 + image: testing-image-2 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" diff --git a/pkg/library/container/testdata/error-has-parent.yaml b/pkg/library/container/testdata/error-has-parent.yaml new file mode 100644 index 000000000..fe9c7d816 --- /dev/null +++ b/pkg/library/container/testdata/error-has-parent.yaml @@ -0,0 +1,12 @@ +name: "Checks for flattened devfile (parent)" + +input: + parent: + registryUrl: "has-parent" + components: + - name: testing-container + container: + image: testing-image + +output: + errRegexp: "devfile is not flattened" diff --git a/pkg/library/container/testdata/error-has-plugins.yaml b/pkg/library/container/testdata/error-has-plugins.yaml new file mode 100644 index 000000000..a94bb44af --- /dev/null +++ b/pkg/library/container/testdata/error-has-plugins.yaml @@ -0,0 +1,13 @@ +name: "Checks for flattened devfile (parent)" + +input: + components: + - name: testing-container + container: + image: testing-image + - name: my-plugin + plugin: + id: "test-id" + +output: + errRegexp: "devfile is not flattened" diff --git a/pkg/library/container/testdata/error-invalid-resources.yaml b/pkg/library/container/testdata/error-invalid-resources.yaml new file mode 100644 index 000000000..6a8c6b426 --- /dev/null +++ b/pkg/library/container/testdata/error-invalid-resources.yaml @@ -0,0 +1,11 @@ +name: "Checks for flattened devfile (parent)" + +input: + components: + - name: testing-container + container: + image: testing-image + memoryLimit: "x" + +output: + errRegexp: "failed to parse memory limit.*" diff --git a/pkg/library/container/testdata/handles-mountSources.yaml b/pkg/library/container/testdata/handles-mountSources.yaml new file mode 100644 index 000000000..c84aeadb5 --- /dev/null +++ b/pkg/library/container/testdata/handles-mountSources.yaml @@ -0,0 +1,60 @@ +name: "Handle mountSources correctly" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + memoryLimit: 999Mi # isolate test to not include memoryLimit + sourceMapping: "/testdir1" + # no mountSources defined -> should mount sources + - name: testing-container-2 + container: + image: testing-image-2 + memoryLimit: 999Mi # isolate test to not include memoryLimit + # No sourceMapping -> defaults to "/projects" + mountSources: true # mountSources: true -> should mount sources + - name: testing-container-3 + container: + image: testing-image-3 + memoryLimit: 999Mi # isolate test to not include memoryLimit + sourceMapping: "/projects" + mountSources: false # mountSources: false -> should not mount sources +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + volumeMounts: + - name: "projects" + mountPath: "/testdir1" + env: + - name: "PROJECTS_ROOT" + value: "/testdir1" + - name: "PROJECTS_SOURCE" + value: "/testdir1" # Temp value until projects is figured out + - name: testing-container-2 + image: testing-image-2 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + volumeMounts: + - name: "projects" + mountPath: "/projects" + env: + - name: "PROJECTS_ROOT" + value: "/projects" + - name: "PROJECTS_SOURCE" + value: "/projects" # Temp value until projects is figured out + + - name: testing-container-3 + image: testing-image-3 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" diff --git a/pkg/library/container/testdata/handles-resources.yaml b/pkg/library/container/testdata/handles-resources.yaml new file mode 100644 index 000000000..35bac169b --- /dev/null +++ b/pkg/library/container/testdata/handles-resources.yaml @@ -0,0 +1,30 @@ +name: "Processes memoryLimit correctly" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + memoryLimit: 999Mi + mountSources: false + - name: testing-container-2 + container: + image: testing-image-2 + # Omitted memory limit - should use default + mountSources: false + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + - name: testing-container-2 + image: testing-image-2 + imagePullPolicy: Always + resources: + limits: + memory: "128M" diff --git a/pkg/library/container/testdata/ignores-non-container-components.yaml b/pkg/library/container/testdata/ignores-non-container-components.yaml new file mode 100644 index 000000000..30252a437 --- /dev/null +++ b/pkg/library/container/testdata/ignores-non-container-components.yaml @@ -0,0 +1,20 @@ +name: "Ignores non-container components" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + mountSources: false # isolate test to not include volumes + memoryLimit: "999Mi" # isolate test to not include memoryLimit + - name: test-volume + volume: {} +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" diff --git a/pkg/library/lifecycle/lifecycle_test.go b/pkg/library/lifecycle/lifecycle_test.go index ac7fb2b2c..5d6924a61 100644 --- a/pkg/library/lifecycle/lifecycle_test.go +++ b/pkg/library/lifecycle/lifecycle_test.go @@ -65,6 +65,9 @@ func TestGetInitContainers(t *testing.T) { if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") } else { + if !assert.NoError(t, err, "Should not return error") { + return + } assert.Equal(t, tt.Output.InitContainers, gotInitContainers, "Init containers should match expected") assert.Equal(t, tt.Output.MainContainers, gotMainComponents, "Main containers should match expected") } diff --git a/pkg/library/storage/commonStorage_test.go b/pkg/library/storage/commonStorage_test.go new file mode 100644 index 000000000..8e3b500d9 --- /dev/null +++ b/pkg/library/storage/commonStorage_test.go @@ -0,0 +1,172 @@ +// +// Copyright (c) 2019-2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package storage + +import ( + "fmt" + "io/ioutil" + "path/filepath" + "testing" + + "github.com/devfile/devworkspace-operator/pkg/config" + corev1 "k8s.io/api/core/v1" + + devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" +) + +type testCase struct { + Name string `json:"name,omitempty"` + Input testInput `json:"input,omitempty"` + Output testOutput `json:"output,omitempty"` +} + +type testInput struct { + WorkspaceID string `json:"workspaceId,omitempty"` + PodAdditions v1alpha1.PodAdditions `json:"podAdditions,omitempty"` + Workspace devworkspace.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` +} + +type testOutput struct { + PodAdditions v1alpha1.PodAdditions `json:"podAdditions,omitempty"` + ErrRegexp *string `json:"errRegexp,omitempty"` +} + +var testControllerCfg = &corev1.ConfigMap{ + Data: map[string]string{ + "devworkspace.sidecar.image_pull_policy": "Always", + }, +} + +func setupControllerCfg() { + config.SetupConfigForTesting(testControllerCfg) +} + +func loadTestCaseOrPanic(t *testing.T, testFilename string) testCase { + testPath := filepath.Join("./testdata", testFilename) + bytes, err := ioutil.ReadFile(testPath) + if err != nil { + t.Fatal(err) + } + var test testCase + if err := yaml.Unmarshal(bytes, &test); err != nil { + t.Fatal(err) + } + t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) + return test +} + +func TestRewriteContainerVolumeMounts(t *testing.T) { + tests := []testCase{ + loadTestCaseOrPanic(t, "does-nothing-for-no-storage-needed.yaml"), + loadTestCaseOrPanic(t, "projects-volume-overriding.yaml"), + loadTestCaseOrPanic(t, "rewrites-volumes-for-common-pvc-strategy.yaml"), + loadTestCaseOrPanic(t, "error-duplicate-volumes.yaml"), + loadTestCaseOrPanic(t, "error-undefined-volume.yaml"), + loadTestCaseOrPanic(t, "error-undefined-volume-init-container.yaml"), + } + setupControllerCfg() + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + // sanity check that file is read correctly. + assert.NotNil(t, tt.Input.Workspace, "Input does not define workspace") + podAdditions := v1alpha1.PodAdditions(tt.Input.PodAdditions) + err := RewriteContainerVolumeMounts(tt.Input.WorkspaceID, &podAdditions, tt.Input.Workspace) + if tt.Output.ErrRegexp != nil && assert.Error(t, err) { + assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") + } else { + if !assert.NoError(t, err, "Should not return error") { + return + } + assert.Equal(t, tt.Output.PodAdditions, podAdditions, "PodAdditions should match expected output") + } + }) + } +} + +func TestNeedsStorage(t *testing.T) { + boolFalse := false + boolTrue := true + tests := []struct { + Name string + Explanation string + Components []devworkspace.ComponentUnion + }{ + { + Name: "Has volume component", + Explanation: "If the devfile has a volume component, it requires storage", + Components: []devworkspace.ComponentUnion{ + { + Volume: &devworkspace.VolumeComponent{}, + }, + }, + }, + { + Name: "Has container component with volume mounts", + Explanation: "If a devfile container has volumeMounts, it requires storage", + Components: []devworkspace.ComponentUnion{ + { + Container: &devworkspace.ContainerComponent{ + Container: devworkspace.Container{ + MountSources: &boolFalse, + VolumeMounts: []devworkspace.VolumeMount{ + { + Name: "test-volumeMount", + }, + }, + }, + }, + }, + }, + }, + { + Name: "Container has mountSources", + Explanation: "If a devfile container has mountSources set, it requires storage", + Components: []devworkspace.ComponentUnion{ + { + Container: &devworkspace.ContainerComponent{ + Container: devworkspace.Container{ + MountSources: &boolTrue, + }, + }, + }, + }, + }, + { + Name: "Container has implicit mountSources", + Explanation: "If a devfile container does not have mountSources set, the default is true", + Components: []devworkspace.ComponentUnion{ + { + Container: &devworkspace.ContainerComponent{ + Container: devworkspace.Container{}, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + workspace := devworkspace.DevWorkspaceTemplateSpec{} + for idx, comp := range tt.Components { + workspace.Components = append(workspace.Components, devworkspace.Component{ + Name: fmt.Sprintf("test-component-%d", idx), + ComponentUnion: comp, + }) + } + assert.True(t, NeedsStorage(workspace), tt.Explanation) + }) + } +} diff --git a/pkg/library/storage/testdata/does-nothing-for-no-storage-needed.yaml b/pkg/library/storage/testdata/does-nothing-for-no-storage-needed.yaml new file mode 100644 index 000000000..3f092d5f0 --- /dev/null +++ b/pkg/library/storage/testdata/does-nothing-for-no-storage-needed.yaml @@ -0,0 +1,22 @@ +name: "Does not modify PodAdditions when storage is not required" + +input: + workspaceId: "test-workspaceid" + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + workspace: + components: + - name: testing-container-1 + container: + image: testing-image-1 + mountSources: false + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always diff --git a/pkg/library/storage/testdata/error-duplicate-volumes.yaml b/pkg/library/storage/testdata/error-duplicate-volumes.yaml new file mode 100644 index 000000000..e63028795 --- /dev/null +++ b/pkg/library/storage/testdata/error-duplicate-volumes.yaml @@ -0,0 +1,27 @@ +name: "Returns error when workspace defines duplicate volumes" + +input: + workspaceId: "test-workspaceid" + podAdditions: + containers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: "projects" + mountPath: "/projects-mountpath" + - name: "my-defined-volume" + mountPath: "/test-1" + + workspace: + components: + - name: testing-container-1 + container: + image: testing-image-1 + sourceMapping: "/plugins-mountpath" + - name: my-defined-volume + volume: {} + - name: my-defined-volume + volume: {} + +output: + errRegexp: "volume component 'my-defined-volume' is defined multiple times" diff --git a/pkg/library/storage/testdata/error-undefined-volume-init-container.yaml b/pkg/library/storage/testdata/error-undefined-volume-init-container.yaml new file mode 100644 index 000000000..364648675 --- /dev/null +++ b/pkg/library/storage/testdata/error-undefined-volume-init-container.yaml @@ -0,0 +1,24 @@ +name: "Returns error on undefined volume in init container" + +input: + workspaceId: "test-workspaceid" + podAdditions: + initContainers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: "projects" + mountPath: "/projects-mountpath" + - name: "my-defined-volume" + mountPath: "/test-1" + + workspace: + components: + - name: testing-container-1 + container: + image: testing-image-1 + sourceMapping: "/plugins-mountpath" + + +output: + errRegexp: "container 'testing-container-1' references undefined volume 'my-defined-volume'" diff --git a/pkg/library/storage/testdata/error-undefined-volume.yaml b/pkg/library/storage/testdata/error-undefined-volume.yaml new file mode 100644 index 000000000..b27258a7d --- /dev/null +++ b/pkg/library/storage/testdata/error-undefined-volume.yaml @@ -0,0 +1,24 @@ +name: "Returns error on undefined volume in container" + +input: + workspaceId: "test-workspaceid" + podAdditions: + containers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: "projects" + mountPath: "/projects-mountpath" + - name: "my-defined-volume" + mountPath: "/test-1" + + workspace: + components: + - name: testing-container-1 + container: + image: testing-image-1 + sourceMapping: "/plugins-mountpath" + + +output: + errRegexp: "container 'testing-container-1' references undefined volume 'my-defined-volume'" diff --git a/pkg/library/storage/testdata/projects-volume-overriding.yaml b/pkg/library/storage/testdata/projects-volume-overriding.yaml new file mode 100644 index 000000000..9500fdd67 --- /dev/null +++ b/pkg/library/storage/testdata/projects-volume-overriding.yaml @@ -0,0 +1,35 @@ +name: "User can specify a projects volume manually" + +input: + workspaceId: "test-workspaceid" + podAdditions: + containers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: "projects" + mountPath: "/projects-mountpath" + + workspace: + components: + - name: testing-container-1 + container: + image: testing-image-1 + sourceMapping: "/plugins-mountpath" + - name: projects + volume: {} + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: claim-devworkspace + subPath: "test-workspaceid/projects" + mountPath: "/projects-mountpath" + + volumes: + - name: claim-devworkspace + persistentVolumeClaim: + claimName: claim-devworkspace diff --git a/pkg/library/storage/testdata/rewrites-volumes-for-common-pvc-strategy.yaml b/pkg/library/storage/testdata/rewrites-volumes-for-common-pvc-strategy.yaml new file mode 100644 index 000000000..5eac4fc66 --- /dev/null +++ b/pkg/library/storage/testdata/rewrites-volumes-for-common-pvc-strategy.yaml @@ -0,0 +1,59 @@ +name: "Rewrites volumeMounts according to common PVC strategy" + +input: + workspaceId: "test-workspaceid" + podAdditions: + containers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: "projects" + mountPath: "/projects-mountpath" + - name: "my-defined-volume" + mountPath: "/test-1" + initContainers: + - name: testing-initContainer-1 + image: testing-image + volumeMounts: + - name: "plugins" + mountPath: "/plugins" + - name: "my-defined-volume" + mountPath: "/test-3" + + workspace: + components: + - name: testing-container-1 + container: + image: testing-image-1 + sourceMapping: "/plugins-mountpath" + - name: my-defined-volume + volume: {} + - name: plugins + volume: {} + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image + volumeMounts: + - name: claim-devworkspace + subPath: "test-workspaceid/projects" + mountPath: "/projects-mountpath" + - name: claim-devworkspace + subPath: "test-workspaceid/my-defined-volume" + mountPath: "/test-1" + initContainers: + - name: testing-initContainer-1 + image: testing-image + volumeMounts: + - name: claim-devworkspace + subPath: "test-workspaceid/plugins" + mountPath: "/plugins" + - name: claim-devworkspace + subPath: "test-workspaceid/my-defined-volume" + mountPath: "/test-3" + volumes: + - name: claim-devworkspace + persistentVolumeClaim: + claimName: claim-devworkspace From e2ed92e9f9d5d40ab9e80d8309c13d20cc6d39fa Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 7 Jan 2021 16:47:03 -0500 Subject: [PATCH 10/14] Fix issue with duplicate containerPort due to webviews endpoint Need to make sure we only add each exposed port once on a container Signed-off-by: Angel Misevski --- pkg/library/container/container_test.go | 1 + pkg/library/container/conversion.go | 5 ++++ .../handles-endpoints-with-common-port.yaml | 30 +++++++++++++++++++ pkg/library/storage/commonStorage_test.go | 5 ++-- 4 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 pkg/library/container/testdata/handles-endpoints-with-common-port.yaml diff --git a/pkg/library/container/container_test.go b/pkg/library/container/container_test.go index 77feff4f5..a1ca560c5 100644 --- a/pkg/library/container/container_test.go +++ b/pkg/library/container/container_test.go @@ -67,6 +67,7 @@ func TestGetKubeContainersFromDevfile(t *testing.T) { loadTestCaseOrPanic(t, "detects-init-containers.yaml"), loadTestCaseOrPanic(t, "handles-mountSources.yaml"), loadTestCaseOrPanic(t, "handles-resources.yaml"), + loadTestCaseOrPanic(t, "handles-endpoints-with-common-port.yaml"), loadTestCaseOrPanic(t, "ignores-non-container-components.yaml"), loadTestCaseOrPanic(t, "converts-all-fields.yaml"), loadTestCaseOrPanic(t, "error-has-parent.yaml"), diff --git a/pkg/library/container/conversion.go b/pkg/library/container/conversion.go index 06d5d3d22..996b48872 100644 --- a/pkg/library/container/conversion.go +++ b/pkg/library/container/conversion.go @@ -49,13 +49,18 @@ func convertContainerToK8s(devfileComponent v1alpha2.Component) (*v1.Container, func devfileEndpointsToContainerPorts(endpoints []v1alpha2.Endpoint) []v1.ContainerPort { var containerPorts []v1.ContainerPort + exposedPorts := map[int]bool{} for _, endpoint := range endpoints { + if exposedPorts[endpoint.TargetPort] { + continue + } containerPorts = append(containerPorts, v1.ContainerPort{ // Use meaningless name for port since endpoint.Name does not match requirements for ContainerPort name Name: fmt.Sprintf("%d-%s", endpoint.TargetPort, endpoint.Protocol), ContainerPort: int32(endpoint.TargetPort), Protocol: v1.ProtocolTCP, }) + exposedPorts[endpoint.TargetPort] = true } return containerPorts } diff --git a/pkg/library/container/testdata/handles-endpoints-with-common-port.yaml b/pkg/library/container/testdata/handles-endpoints-with-common-port.yaml new file mode 100644 index 000000000..6f116f3c6 --- /dev/null +++ b/pkg/library/container/testdata/handles-endpoints-with-common-port.yaml @@ -0,0 +1,30 @@ +name: "Handles container with multiple endpoints with same targetPort" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + memoryLimit: 999Mi # isolate test to not include memoryLimit + mountSources: false + endpoints: + - name: "test-endpoint-1" + targetPort: 3100 + protocol: http + - name: "test-endpoint-2" + targetPort: 3100 + protocol: http + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + ports: + - name: "3100-http" + containerPort: 3100 + protocol: TCP diff --git a/pkg/library/storage/commonStorage_test.go b/pkg/library/storage/commonStorage_test.go index 8e3b500d9..39b7f7d50 100644 --- a/pkg/library/storage/commonStorage_test.go +++ b/pkg/library/storage/commonStorage_test.go @@ -83,15 +83,14 @@ func TestRewriteContainerVolumeMounts(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { // sanity check that file is read correctly. assert.NotNil(t, tt.Input.Workspace, "Input does not define workspace") - podAdditions := v1alpha1.PodAdditions(tt.Input.PodAdditions) - err := RewriteContainerVolumeMounts(tt.Input.WorkspaceID, &podAdditions, tt.Input.Workspace) + err := RewriteContainerVolumeMounts(tt.Input.WorkspaceID, &tt.Input.PodAdditions, tt.Input.Workspace) if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") } else { if !assert.NoError(t, err, "Should not return error") { return } - assert.Equal(t, tt.Output.PodAdditions, podAdditions, "PodAdditions should match expected output") + assert.Equal(t, tt.Output.PodAdditions, tt.Input.PodAdditions, "PodAdditions should match expected output") } }) } From 6ed526e4de7b9d5e6ade4b79ff9c708c64bfce2b Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Mon, 11 Jan 2021 11:04:37 -0500 Subject: [PATCH 11/14] Use flattened web-terminal devworkspace in e2e tests Signed-off-by: Angel Misevski --- test/e2e/pkg/tests/devworkspaces_tests.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/pkg/tests/devworkspaces_tests.go b/test/e2e/pkg/tests/devworkspaces_tests.go index 9c7afd16a..3bdc07318 100644 --- a/test/e2e/pkg/tests/devworkspaces_tests.go +++ b/test/e2e/pkg/tests/devworkspaces_tests.go @@ -40,7 +40,8 @@ var _ = ginkgo.Describe("[Create OpenShift Web Terminal Workspace]", func() { }) ginkgo.It("Add OpenShift web terminal to cluster and wait running status", func() { - commandResult, err := config.DevK8sClient.OcApplyWorkspace(config.WorkspaceNamespace, "samples/web-terminal.yaml") + // TODO#185 : Temporarily use pre-flattened devworkspace until flattening is re-implemented + commandResult, err := config.DevK8sClient.OcApplyWorkspace(config.WorkspaceNamespace, "samples/flattened_web-terminal.yaml") if err != nil { ginkgo.Fail(fmt.Sprintf("Failed to create OpenShift web terminal workspace: %s %s", err.Error(), commandResult)) return From de51953f135157b765330ddddfa15ee54b448573 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 15 Jan 2021 11:50:28 -0500 Subject: [PATCH 12/14] Don't add projects volumeMount if container mounts it manually This avoids an issue where a container can mount the projects volume directly and have `mountSources: true`, which results in a duplicate mount and invalid deployment. Related to issue https://github.com/devfile/api/issues/290 and how projects volume should be handled with `mountSources`. Signed-off-by: Angel Misevski --- pkg/library/container/container_test.go | 5 ++- pkg/library/container/mountSources.go | 36 ++++++++++++++----- ...ntainer-that-mounts-projects-directly.yaml | 34 ++++++++++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 pkg/library/container/testdata/handles-container-that-mounts-projects-directly.yaml diff --git a/pkg/library/container/container_test.go b/pkg/library/container/container_test.go index a1ca560c5..85d460836 100644 --- a/pkg/library/container/container_test.go +++ b/pkg/library/container/container_test.go @@ -18,6 +18,7 @@ import ( "path/filepath" "testing" + "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2" @@ -68,6 +69,7 @@ func TestGetKubeContainersFromDevfile(t *testing.T) { loadTestCaseOrPanic(t, "handles-mountSources.yaml"), loadTestCaseOrPanic(t, "handles-resources.yaml"), loadTestCaseOrPanic(t, "handles-endpoints-with-common-port.yaml"), + loadTestCaseOrPanic(t, "handles-container-that-mounts-projects-directly.yaml"), loadTestCaseOrPanic(t, "ignores-non-container-components.yaml"), loadTestCaseOrPanic(t, "converts-all-fields.yaml"), loadTestCaseOrPanic(t, "error-has-parent.yaml"), @@ -87,7 +89,8 @@ func TestGetKubeContainersFromDevfile(t *testing.T) { if !assert.NoError(t, err, "Should not return error") { return } - assert.Equal(t, tt.Output.PodAdditions, gotPodAdditions, "PodAdditions should match expected output") + assert.True(t, cmp.Equal(tt.Output.PodAdditions, gotPodAdditions), + "PodAdditions should match expected output: \n%s", cmp.Diff(tt.Output.PodAdditions, gotPodAdditions)) } }) } diff --git a/pkg/library/container/mountSources.go b/pkg/library/container/mountSources.go index 0c93b021a..5156894f8 100644 --- a/pkg/library/container/mountSources.go +++ b/pkg/library/container/mountSources.go @@ -40,16 +40,23 @@ func handleMountSources(k8sContainer *corev1.Container, devfileContainer *devwor if !HasMountSources(devfileContainer) { return } - sourceMapping := devfileContainer.SourceMapping - if sourceMapping == "" { - // Sanity check -- this value should be defaulted to `/projects` but may not be - // if struct was not processed by k8s - sourceMapping = config.DefaultProjectsSourcesRoot + var sourceMapping string + if vm := getProjectsVolumeMount(k8sContainer); vm != nil { + // Container already mounts projects volume; need to set env vars according to mountPath + // TODO: see issue https://github.com/devfile/api/issues/290 + sourceMapping = vm.MountPath + } else { + sourceMapping = devfileContainer.SourceMapping + if sourceMapping == "" { + // Sanity check -- this value should be defaulted to `/projects` but may not be + // if struct was not processed by k8s + sourceMapping = config.DefaultProjectsSourcesRoot + } + k8sContainer.VolumeMounts = append(k8sContainer.VolumeMounts, corev1.VolumeMount{ + Name: constants.ProjectsVolumeName, + MountPath: sourceMapping, + }) } - k8sContainer.VolumeMounts = append(k8sContainer.VolumeMounts, corev1.VolumeMount{ - Name: constants.ProjectsVolumeName, - MountPath: sourceMapping, - }) k8sContainer.Env = append(k8sContainer.Env, corev1.EnvVar{ Name: constants.ProjectsRootEnvVar, Value: sourceMapping, @@ -58,3 +65,14 @@ func handleMountSources(k8sContainer *corev1.Container, devfileContainer *devwor Value: sourceMapping, // TODO: Unclear how this should be handled in case of multiple projects }) } + +// getProjectsVolumeMount returns the projects volumeMount in a container, if it is defined; if it does not exist, +// returns nil. +func getProjectsVolumeMount(k8sContainer *corev1.Container) *corev1.VolumeMount { + for _, vm := range k8sContainer.VolumeMounts { + if vm.Name == constants.ProjectsVolumeName { + return &vm + } + } + return nil +} diff --git a/pkg/library/container/testdata/handles-container-that-mounts-projects-directly.yaml b/pkg/library/container/testdata/handles-container-that-mounts-projects-directly.yaml new file mode 100644 index 000000000..3256ca971 --- /dev/null +++ b/pkg/library/container/testdata/handles-container-that-mounts-projects-directly.yaml @@ -0,0 +1,34 @@ +# This test is due to an inconsistency in devfile/api semantics; see +# issue https://github.com/devfile/api/issues/290 + +name: "Handles container with mountSources and explicit projects volume" + +input: + components: + - name: testing-container-1 + container: + image: testing-image-1 + memoryLimit: 999Mi # isolate test to not include memoryLimit + sourceMapping: "/testdir1" + mountSources: true + volumeMounts: + - name: "projects" + path: "/not-source-mapping" + +output: + podAdditions: + containers: + - name: testing-container-1 + image: testing-image-1 + imagePullPolicy: Always + resources: + limits: + memory: "999Mi" + volumeMounts: + - name: "projects" + mountPath: "/not-source-mapping" + env: + - name: "PROJECTS_ROOT" + value: "/not-source-mapping" + - name: "PROJECTS_SOURCE" + value: "/not-source-mapping" # Temp value until projects is figured out From 3d1c1f0785520119b99ac299893db50d51ccdeb0 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 19 Jan 2021 10:58:26 -0500 Subject: [PATCH 13/14] Fix issue where volumeMounts were being unnecessarily added Improve check for whether the volumeMount subpaths workaround is required for a particular deployment. Previously, we only checked whether any volumes were defined, which failed on OpenShift due to a secret volume being in the list. Now we check to make sure the common PVC volume is present in the deployment before applying the workaround. Signed-off-by: Angel Misevski --- controllers/workspace/provision/deployment.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/controllers/workspace/provision/deployment.go b/controllers/workspace/provision/deployment.go index 9f079551f..feee34cba 100644 --- a/controllers/workspace/provision/deployment.go +++ b/controllers/workspace/provision/deployment.go @@ -223,7 +223,7 @@ func getSpecDeployment( }, } - if len(podAdditions.Volumes) > 0 { + if needsPVCWorkaround(podAdditions) { // Kubernetes creates directories in a PVC to support subpaths such that only the leaf directory has g+rwx permissions. // This means that mounting the subpath e.g. /plugins will result in the directory being // created with 755 permissions, requiring the root UID to remove it. @@ -353,3 +353,13 @@ func getWorkspaceSubpathVolumeMount(workspaceId string) corev1.VolumeMount { return workspaceVolumeMount } + +func needsPVCWorkaround(podAdditions *v1alpha1.PodAdditions) bool { + commonPVCName := config.ControllerCfg.GetWorkspacePVCName() + for _, vol := range podAdditions.Volumes { + if vol.Name == commonPVCName { + return true + } + } + return false +} From ce39b2602843e4581160166613c6344018f57e93 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 12 Jan 2021 22:59:10 -0500 Subject: [PATCH 14/14] Add function to populate env vars required by machine-exec Machine exec depends on the CHE_MACHINE_NAME plugin to find the right container to exec into. Add a function for populating env vars to the shim library. Signed-off-by: Angel Misevski --- controllers/workspace/devworkspace_controller.go | 1 + pkg/library/shim/component.go | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 4b624ad92..4fecdb124 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -188,6 +188,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile volumes: %s", err) return reconcile.Result{}, nil } + shimlib.FillDefaultEnvVars(devfilePodAdditions, workspace.Spec.Template) componentDescriptions, err := shimlib.GetComponentDescriptionsFromPodAdditions(devfilePodAdditions, workspace.Spec.Template) if err != nil { reqLogger.Info("DevWorkspace start failed") diff --git a/pkg/library/shim/component.go b/pkg/library/shim/component.go index bf77c4752..900ac0031 100644 --- a/pkg/library/shim/component.go +++ b/pkg/library/shim/component.go @@ -26,6 +26,15 @@ import ( corev1 "k8s.io/api/core/v1" ) +func FillDefaultEnvVars(podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) { + for idx, mainContainer := range podAdditions.Containers { + podAdditions.Containers[idx].Env = append(mainContainer.Env, corev1.EnvVar{ + Name: "CHE_MACHINE_NAME", + Value: mainContainer.Name, + }) + } +} + func GetComponentDescriptionsFromPodAdditions(podAdditions *v1alpha1.PodAdditions, workspace devworkspace.DevWorkspaceTemplateSpec) ([]v1alpha1.ComponentDescription, error) { var descriptions []v1alpha1.ComponentDescription