From ecf6925a5f304ae9b813bd01ca870956e4b06d5b Mon Sep 17 00:00:00 2001 From: deviantony Date: Fri, 22 Sep 2023 07:24:05 +0000 Subject: [PATCH 1/3] fix: review label usage for pvc and volume association --- .../adapter/converter/persistentvolumeclaim.go | 2 +- internal/adapter/deployment.go | 2 +- internal/adapter/filters/filters.go | 12 ++++++------ internal/adapter/persistentvolumeclaim.go | 12 +++++++----- internal/adapter/store/filesystem/configmap.go | 7 ++++--- internal/adapter/store/filesystem/secret.go | 7 ++++--- internal/adapter/store/filesystem/store.go | 5 ----- internal/adapter/types/labels.go | 18 ++++++++++++++++-- 8 files changed, 39 insertions(+), 26 deletions(-) diff --git a/internal/adapter/converter/persistentvolumeclaim.go b/internal/adapter/converter/persistentvolumeclaim.go index 0159352..fecc7bf 100644 --- a/internal/adapter/converter/persistentvolumeclaim.go +++ b/internal/adapter/converter/persistentvolumeclaim.go @@ -17,7 +17,7 @@ func (converter *DockerAPIConverter) UpdateConfigMapToPersistentVolumeClaim(pers persistentVolumeClaim.ObjectMeta = metav1.ObjectMeta{ Name: configMap.Labels[k2dtypes.PersistentVolumeClaimNameLabelKey], - Namespace: configMap.Labels[k2dtypes.NamespaceNameLabelKey], + Namespace: configMap.Labels[k2dtypes.PersistentVolumeClaimTargetNamespaceLabelKey], CreationTimestamp: metav1.Time{ Time: configMap.CreationTimestamp.Time, }, diff --git a/internal/adapter/deployment.go b/internal/adapter/deployment.go index fc45ee5..7d27c78 100644 --- a/internal/adapter/deployment.go +++ b/internal/adapter/deployment.go @@ -24,7 +24,7 @@ func (adapter *KubeDockerAdapter) CreateContainerFromDeployment(ctx context.Cont labels: deployment.Spec.Template.Labels, } - opts.labels[k2dtypes.WorkloadLabelKey] = k2dtypes.DeploymentWorkloadType + opts.labels[k2dtypes.WorkloadTypeLabelKey] = k2dtypes.DeploymentWorkloadType if deployment.Labels["app.kubernetes.io/managed-by"] == "Helm" { deploymentData, err := json.Marshal(deployment) diff --git a/internal/adapter/filters/filters.go b/internal/adapter/filters/filters.go index 748e2a3..e8210bc 100644 --- a/internal/adapter/filters/filters.go +++ b/internal/adapter/filters/filters.go @@ -22,7 +22,7 @@ import ( // // Now 'filter' can be used in Docker API calls to filter Deployment resources in the 'default' Kubernetes namespace. func AllDeployments(namespace string) filters.Args { filter := ByNamespace(namespace) - filter.Add("label", fmt.Sprintf("%s=%s", types.WorkloadLabelKey, types.DeploymentWorkloadType)) + filter.Add("label", fmt.Sprintf("%s=%s", types.WorkloadTypeLabelKey, types.DeploymentWorkloadType)) return filter } @@ -141,19 +141,19 @@ func ByService(namespace, serviceName string) filters.Args { return filter } -// AllPersistentVolumes creates a Docker filter argument that targets resources labeled with a Kubernetes persistent volume name. -// This function uses the types.PersistentVolumeNameLabelKey constant as the base label key to filter Docker resources. +// AllPersistentVolumes creates a Docker filter argument that targets resources labeled with a specific type of storage, in this case, Kubernetes persistent volumes. +// This function uses the types.StorageTypeLabelKey and types.PersistentVolumeStorageType constants to filter Docker resources. // // Parameters: // - None // // Returns: -// - filters.Args: A Docker filter object that can be used to filter Docker API calls based on the presence of the persistent volume name label. +// - filters.Args: A Docker filter object that can be used to filter Docker API calls based on the presence of a label indicating the storage type as a Kubernetes persistent volume. // // Usage Example: // // filter := AllPersistentVolumes() -// // Now 'filter' can be used in Docker API calls to filter resources that are labeled with any Kubernetes persistent volume name. +// // Now 'filter' can be used in Docker API calls to filter resources that are labeled with a specific type of storage as a Kubernetes persistent volume. func AllPersistentVolumes() filters.Args { - return filters.NewArgs(filters.Arg("label", types.PersistentVolumeNameLabelKey)) + return filters.NewArgs(filters.Arg("label", fmt.Sprintf("%s=%s", types.StorageTypeLabelKey, types.PersistentVolumeStorageType))) } diff --git a/internal/adapter/persistentvolumeclaim.go b/internal/adapter/persistentvolumeclaim.go index a0b5ac6..230b599 100644 --- a/internal/adapter/persistentvolumeclaim.go +++ b/internal/adapter/persistentvolumeclaim.go @@ -65,6 +65,7 @@ func (adapter *KubeDockerAdapter) CreatePersistentVolumeClaim(ctx context.Contex Driver: "local", Labels: map[string]string{ k2dtypes.PersistentVolumeNameLabelKey: volumeName, + k2dtypes.StorageTypeLabelKey: k2dtypes.PersistentVolumeStorageType, }, }) @@ -85,13 +86,14 @@ func (adapter *KubeDockerAdapter) CreatePersistentVolumeClaim(ctx context.Contex ObjectMeta: metav1.ObjectMeta{ Name: naming.BuildPVCSystemConfigMapName(persistentVolumeClaim.Name, persistentVolumeClaim.Namespace), Labels: map[string]string{ - k2dtypes.NamespaceNameLabelKey: persistentVolumeClaim.Namespace, - k2dtypes.PersistentVolumeNameLabelKey: volumeName, - k2dtypes.PersistentVolumeClaimNameLabelKey: persistentVolumeClaim.Name, - k2dtypes.LastAppliedConfigLabelKey: persistentVolumeClaim.ObjectMeta.Annotations["kubectl.kubernetes.io/last-applied-configuration"], + k2dtypes.PersistentVolumeNameLabelKey: volumeName, + k2dtypes.PersistentVolumeClaimNameLabelKey: persistentVolumeClaim.Name, + k2dtypes.PersistentVolumeClaimTargetNamespaceLabelKey: persistentVolumeClaim.Namespace, + k2dtypes.LastAppliedConfigLabelKey: persistentVolumeClaim.ObjectMeta.Annotations["kubectl.kubernetes.io/last-applied-configuration"], }, }, } + err := adapter.CreateSystemConfigMap(pvcConfigMap) if err != nil { return fmt.Errorf("unable to create system configmap for persistent volume claim: %w", err) @@ -203,7 +205,7 @@ func (adapter *KubeDockerAdapter) listPersistentVolumeClaims(ctx context.Context } for _, configMap := range configMaps.Items { - namespace := configMap.Labels[k2dtypes.NamespaceNameLabelKey] + namespace := configMap.Labels[k2dtypes.PersistentVolumeClaimTargetNamespaceLabelKey] if namespaceName == "" || namespace == namespaceName { pvcLastAppliedConfig := configMap.Labels[k2dtypes.LastAppliedConfigLabelKey] diff --git a/internal/adapter/store/filesystem/configmap.go b/internal/adapter/store/filesystem/configmap.go index b4478f1..bcc96ce 100644 --- a/internal/adapter/store/filesystem/configmap.go +++ b/internal/adapter/store/filesystem/configmap.go @@ -8,6 +8,7 @@ import ( "time" "github.com/portainer/k2d/internal/adapter/errors" + "github.com/portainer/k2d/internal/adapter/types" "github.com/portainer/k2d/pkg/filesystem" "github.com/portainer/k2d/pkg/maputils" corev1 "k8s.io/api/core/v1" @@ -207,8 +208,8 @@ func (s *FileSystemStore) StoreConfigMap(configMap *corev1.ConfigMap) error { defer s.mutex.Unlock() labels := map[string]string{ - NamespaceNameLabelKey: configMap.Namespace, - CreationTimestampLabelKey: time.Now().UTC().Format(time.RFC3339), + types.NamespaceNameLabelKey: configMap.Namespace, + CreationTimestampLabelKey: time.Now().UTC().Format(time.RFC3339), } maputils.MergeMapsInPlace(labels, configMap.Labels) @@ -296,7 +297,7 @@ func (s *FileSystemStore) loadMetadataAndInitConfigMaps(metadataFiles []string, return configMaps, fmt.Errorf("unable to load configmap metadata from disk: %w", err) } - namespaceName := metadata[NamespaceNameLabelKey] + namespaceName := metadata[types.NamespaceNameLabelKey] if namespace != "" && namespace != namespaceName { continue } diff --git a/internal/adapter/store/filesystem/secret.go b/internal/adapter/store/filesystem/secret.go index a7f5a62..2fc85f4 100644 --- a/internal/adapter/store/filesystem/secret.go +++ b/internal/adapter/store/filesystem/secret.go @@ -8,6 +8,7 @@ import ( "time" "github.com/portainer/k2d/internal/adapter/errors" + "github.com/portainer/k2d/internal/adapter/types" "github.com/portainer/k2d/pkg/filesystem" "github.com/portainer/k2d/pkg/maputils" corev1 "k8s.io/api/core/v1" @@ -213,8 +214,8 @@ func (s *FileSystemStore) StoreSecret(secret *corev1.Secret) error { defer s.mutex.Unlock() labels := map[string]string{ - NamespaceNameLabelKey: secret.Namespace, - CreationTimestampLabelKey: time.Now().UTC().Format(time.RFC3339), + types.NamespaceNameLabelKey: secret.Namespace, + CreationTimestampLabelKey: time.Now().UTC().Format(time.RFC3339), } maputils.MergeMapsInPlace(labels, secret.Labels) @@ -316,7 +317,7 @@ func (s *FileSystemStore) loadMetadataAndInitSecrets(metadataFiles []string, nam continue } - namespaceName := metadata[NamespaceNameLabelKey] + namespaceName := metadata[types.NamespaceNameLabelKey] if namespace != "" && namespace != namespaceName { continue } diff --git a/internal/adapter/store/filesystem/store.go b/internal/adapter/store/filesystem/store.go index 19650e1..734cb61 100644 --- a/internal/adapter/store/filesystem/store.go +++ b/internal/adapter/store/filesystem/store.go @@ -24,11 +24,6 @@ const ( ) const ( - // NamespaceNameLabelKey is the key used to store the namespace of a Configmap or Secret resource - // in the associated metadata file - // It is used to identify the namespace associated with a ConfigMap or a Secret - NamespaceNameLabelKey = "store.k2d.io/filesystem/namespace-name" - // CreationTimestampLabelKey is the key used to store the creation timestamp of a Configmap or Secret resource // in the associated metadata file CreationTimestampLabelKey = "store.k2d.io/filesystem/creation-timestamp" diff --git a/internal/adapter/types/labels.go b/internal/adapter/types/labels.go index 7d19f26..34932b4 100644 --- a/internal/adapter/types/labels.go +++ b/internal/adapter/types/labels.go @@ -27,14 +27,28 @@ const ( // PersistentVolumeNameLabelKey is the key used to store the persistent volume name in the labels of a system configmap or a Docker volume PersistentVolumeNameLabelKey = "storage.k2d.io/pv-name" + + // PersistentVolumeClaimTargetNamespaceLabelKey is the key used to store the target namespace of a persistent volume claim in the labels of a system configmap + // This is used to identify the namespace where the persistent volume claim is used (e.g. the namespace of the workload) + PersistentVolumeClaimTargetNamespaceLabelKey = "storage.k2d.io/pvc-target-namespace" + + // StorageTypeLabelKey is the key used to store the storage type in the labels of a system configmap or a Docker volume + // It is used to differentiate between persistent volumes and config maps when listing volumes + StorageTypeLabelKey = "storage.k2d.io/type" +) + +const ( + // PersistentVolumeStorageType is the label value used to identify a persistent volume storage + // It is stored inside metadata as a label and used to filter persistent volumes when listing persistent volumes + PersistentVolumeStorageType = "pv" ) const ( // ServiceNameLabelKey is the key used to store the service name associated to the workload in the container labels ServiceNameLabelKey = "workload.k2d.io/service-name" - // WorkloadLabelKey is the key used to store the workload type in the container labels - WorkloadLabelKey = "workload.k2d.io/type" + // WorkloadTypeLabelKey is the key used to store the workload type in the container labels + WorkloadTypeLabelKey = "workload.k2d.io/type" // WorkloadNameLabelKey is the key used to store the workload name in the container labels WorkloadNameLabelKey = "workload.k2d.io/name" From fd8aec7d8bb6480acda493e8b06a8524d59ccd5f Mon Sep 17 00:00:00 2001 From: deviantony Date: Fri, 22 Sep 2023 07:32:50 +0000 Subject: [PATCH 2/3] fix: fix PV namespace resolution --- internal/adapter/converter/persistentvolume.go | 2 +- internal/adapter/persistentvolume.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/adapter/converter/persistentvolume.go b/internal/adapter/converter/persistentvolume.go index 32b0e39..bae4b3f 100644 --- a/internal/adapter/converter/persistentvolume.go +++ b/internal/adapter/converter/persistentvolume.go @@ -24,7 +24,7 @@ func (converter *DockerAPIConverter) ConvertVolumeToPersistentVolume(volume *vol phase = core.VolumeBound persistentVolumeClaimReference = &core.ObjectReference{ Kind: "PersistentVolumeClaim", - Namespace: pvcConfigMap.Labels[k2dtypes.NamespaceNameLabelKey], + Namespace: pvcConfigMap.Labels[k2dtypes.PersistentVolumeClaimTargetNamespaceLabelKey], Name: pvcConfigMap.Labels[k2dtypes.PersistentVolumeClaimNameLabelKey], } } diff --git a/internal/adapter/persistentvolume.go b/internal/adapter/persistentvolume.go index 49b33fc..3358c4f 100644 --- a/internal/adapter/persistentvolume.go +++ b/internal/adapter/persistentvolume.go @@ -40,7 +40,7 @@ func (adapter *KubeDockerAdapter) GetPersistentVolume(ctx context.Context, persi var boundPVCConfigMap *corev1.ConfigMap for _, configMap := range configMaps.Items { - if configMap.Data[k2dtypes.PersistentVolumeNameLabelKey] == volume.Name { + if configMap.Labels[k2dtypes.PersistentVolumeNameLabelKey] == volume.Name { boundPVCConfigMap = &configMap break } From cd6386dd6478bc7c1248d630f9cd67b20b3df2c6 Mon Sep 17 00:00:00 2001 From: deviantony Date: Fri, 22 Sep 2023 07:39:08 +0000 Subject: [PATCH 3/3] docs: update function comments --- internal/adapter/filters/filters.go | 2 +- internal/adapter/persistentvolumeclaim.go | 34 +++++++++++------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/adapter/filters/filters.go b/internal/adapter/filters/filters.go index e8210bc..12d66cc 100644 --- a/internal/adapter/filters/filters.go +++ b/internal/adapter/filters/filters.go @@ -8,7 +8,7 @@ import ( ) // AllDeployments creates a Docker filter argument for Kubernetes Deployments within a given namespace. -// The function filters Docker resources based on the Workload and Namespace labels, specifically for Deployments. +// The function filters Docker resources based on the workload type and namespace labels, specifically for Deployments. // // Parameters: // - namespace: The Kubernetes namespace to filter by. diff --git a/internal/adapter/persistentvolumeclaim.go b/internal/adapter/persistentvolumeclaim.go index 230b599..3c915bd 100644 --- a/internal/adapter/persistentvolumeclaim.go +++ b/internal/adapter/persistentvolumeclaim.go @@ -17,34 +17,34 @@ import ( // CreatePersistentVolumeClaim handles the creation or assignment of a Docker volume for a Kubernetes PersistentVolumeClaim (PVC). // // Parameters: -// - ctx: A context for managing the lifetime of the request. -// - persistentVolumeClaim: A pointer to a Kubernetes PersistentVolumeClaim object that describes the claim. +// - ctx: Context for managing the lifetime of the request. +// - persistentVolumeClaim: Pointer to a Kubernetes PersistentVolumeClaim object describing the desired claim. // // Returns: -// - An error if any step in the creation or assignment process fails. +// - An error if any step in the creation, inspection, or labeling process fails. // // Behavior: // // - Static Volume Assignment: -// If the `Spec.VolumeName` field of the PVC is not empty, the function assumes that this is a static assignment of an existing Docker volume to a PVC. -// 1. Inspects the Docker volume to ensure it exists. -// 2. If the volume does not exist, returns an error. +// If the PVC's `Spec.VolumeName` is not empty, the function assumes a static assignment to an existing Docker volume. +// 1. Inspects the existing Docker volume to verify it exists. +// 2. Returns an error if the volume does not exist. // // - Dynamic Volume Creation: -// If the `Spec.VolumeName` field of the PVC is empty, the function creates a new Docker volume. -// 1. Generates a dynamic name for the Docker volume based on the PVC name and namespace. -// 2. Creates a new Docker volume with the generated name. -// 3. Labels the Docker volume to identify it as a k2d-managed volume. (See `k2dtypes.PersistentVolumeNameLabelKey`) +// If the PVC's `Spec.VolumeName` is empty, the function dynamically creates a Docker volume. +// 1. Generates a name for the Docker volume based on the PVC's name and namespace. +// 2. Creates the Docker volume with the generated name. +// 3. Labels the volume with k2d-specific labels for identification (See `k2dtypes.StorageTypeLabelKey` and `k2dtypes.PersistentVolumeNameLabelKey`). // // - Helm-managed PVCs: -// If the PVC has a label "app.kubernetes.io/managed-by" set to "Helm", it serializes the PVC and stores it in an annotation for later use. +// If the PVC has a label "app.kubernetes.io/managed-by" set to "Helm," the PVC's state is serialized and stored as an annotation for later use. // // - ConfigMap Creation: -// Creates a ConfigMap that represents system-level information for the PVC, which includes: -// 1. The namespace of the PVC. -// 2. The name of the Docker volume. -// 3. The name of the PVC. -// 4. The last applied configuration of the PVC +// Creates a ConfigMap that represents system-level metadata about the PVC, which includes: +// 1. The target namespace of the PVC. +// 2. The name of the corresponding Docker volume. +// 3. The name of the PVC itself. +// 4. The last-applied configuration of the PVC, if available. func (adapter *KubeDockerAdapter) CreatePersistentVolumeClaim(ctx context.Context, persistentVolumeClaim *corev1.PersistentVolumeClaim) error { var volumeName string @@ -64,8 +64,8 @@ func (adapter *KubeDockerAdapter) CreatePersistentVolumeClaim(ctx context.Contex Name: volumeName, Driver: "local", Labels: map[string]string{ - k2dtypes.PersistentVolumeNameLabelKey: volumeName, k2dtypes.StorageTypeLabelKey: k2dtypes.PersistentVolumeStorageType, + k2dtypes.PersistentVolumeNameLabelKey: volumeName, }, })