Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for per-namespace configuration and add setting for workspace PVC request size #487

Merged
merged 3 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/constants/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,8 @@ const (
// Only secrets with 'true' value will be mount as pull secret
// Should be assigned to secrets with type docker config types (kubernetes.io/dockercfg and kubernetes.io/dockerconfigjson)
DevWorkspacePullSecretLabel = "controller.devfile.io/devworkspace_pullsecret"

// NamespacedConfigLabelKey is a label applied to configmaps to mark them as a configuration for all DevWorkspaces in
// the current namespace.
NamespacedConfigLabelKey = "controller.devfile.io/namespaced-config"
)
71 changes: 71 additions & 0 deletions pkg/provision/config/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
//
// Copyright (c) 2019-2021 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 config

import (
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/devfile/devworkspace-operator/controllers/workspace/provision"
"github.com/devfile/devworkspace-operator/pkg/constants"
)

const (
commonPVCSizeKey = "commonPVCSize"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, during testing I found more config we may want to configure, but I'm not sure.
Like pvc name. For CRW it's claim-che-workspace, not sure if CRW expects Che Workspace and DevWorkspaces uses different PVCs.

Also, see che.infra.kubernetes.pvc.storage_class_name.

it's out of the current PR scope for sure, but WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea was that the config struct could be extended to include additional information as needed (storage class makes sense).

For common PVC name, I think it's a limited use case that would be complex to support -- e.g. what happens if it is updated after claim-che-workspace has been created? It seems like it would be an open door for leaking storage after workspaces are deleted, etc.

)

type NamespacedConfig struct {
CommonPVCSize string
}

// ReadNamespacedConfig reads the per-namespace DevWorkspace configmap and returns it as a struct. If there are
// no valid configmaps in the specified namespace, returns (nil, nil). If there are multiple configmaps with the
// per-namespace configmap label, returns an error.
func ReadNamespacedConfig(namespace string, api provision.ClusterAPI) (*NamespacedConfig, error) {
cmList := &corev1.ConfigMapList{}
labelSelector, err := labels.Parse(fmt.Sprintf("%s=true", constants.NamespacedConfigLabelKey))
if err != nil {
return nil, err
}
selector := &client.ListOptions{
Namespace: namespace,
LabelSelector: labelSelector,
}
err = api.Client.List(api.Ctx, cmList, selector)
if err != nil {
return nil, err
}
cms := cmList.Items
if len(cms) == 0 {
return nil, nil
} else if len(cms) > 1 {
var cmNames []string
for _, cm := range cms {
cmNames = append(cmNames, cm.Name)
}
return nil, fmt.Errorf("multiple per-namespace configs found: %s", strings.Join(cmNames, ", "))
}

cm := cms[0]
if cm.Data == nil {
return nil, nil
}

return &NamespacedConfig{
CommonPVCSize: cm.Data[commonPVCSizeKey],
}, nil
}
2 changes: 1 addition & 1 deletion pkg/provision/storage/commonStorage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func TestRewriteContainerVolumeMountsForCommonStorageClass(t *testing.T) {
tests := loadAllTestCasesOrPanic(t, "testdata/common-storage")
setupControllerCfg()
commonStorage := CommonStorageProvisioner{}
commonPVC, err := getCommonPVCSpec("test-namespace")
commonPVC, err := getCommonPVCSpec("test-namespace", "1Gi")
if err != nil {
t.Fatalf("Failure during setup: %s", err)
}
Expand Down
16 changes: 13 additions & 3 deletions pkg/provision/storage/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ import (
"github.com/devfile/devworkspace-operator/pkg/constants"
devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants"
containerlib "github.com/devfile/devworkspace-operator/pkg/library/container"
nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func getCommonPVCSpec(namespace string) (*corev1.PersistentVolumeClaim, error) {
pvcStorageQuantity, err := resource.ParseQuantity(constants.PVCStorageSize)
func getCommonPVCSpec(namespace string, size string) (*corev1.PersistentVolumeClaim, error) {
pvcStorageQuantity, err := resource.ParseQuantity(size)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -79,7 +80,16 @@ func needsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool {
}

func syncCommonPVC(namespace string, clusterAPI provision.ClusterAPI) (*corev1.PersistentVolumeClaim, error) {
pvc, err := getCommonPVCSpec(namespace)
namespacedConfig, err := nsconfig.ReadNamespacedConfig(namespace, clusterAPI)
if err != nil {
return nil, fmt.Errorf("failed to read namespace-specific configuration: %w", err)
}
pvcSize := constants.PVCStorageSize
if namespacedConfig != nil && namespacedConfig.CommonPVCSize != "" {
pvcSize = namespacedConfig.CommonPVCSize
}

pvc, err := getCommonPVCSpec(namespace, pvcSize)
if err != nil {
return nil, err
}
Expand Down