Skip to content

Commit

Permalink
refactor (jkube-kit/enricher) : Move storageClass related functionali…
Browse files Browse the repository at this point in the history
…ty out of VolumePermissionEnricher (eclipse-jkube#1179)

Related to eclipse-jkube#1179

+ Move setting PersistentVolumeClaim's storageClass related logic into a
 new enricher : PersistentVolumeClaimStorageClassEnricher . This way user can
 simply exclude VolumePermissionEnricher to opt out of volume permission
 initContainer.
+ Deprecate VolumePermissionEnricher's `defaultStorageClass` and
  `useStorageClassAnnotation` fields in favor of
  PersistentVolumeClaimStorageClassEnricher equivalents.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed Mar 1, 2023
1 parent fed7f07 commit 63b4cf5
Show file tree
Hide file tree
Showing 13 changed files with 245 additions and 73 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Usage:
./scripts/extract-changelog-for-version.sh 1.3.37 5
```
### 1.12-SNAPSHOT
* Fix #1179: Move storageClass related functionality out of VolumePermissionEnricher to PersistentVolumeClaimStorageClassEnricher
* Fix #1273: Deprecate `jkube.io` annotation prefix in favor of `jkube.eclipse.org` for JKubeAnnotations

### 1.11.0 (2023-02-16)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
# TODO: Documents and verify enrichers below
- jkube-debug
- jkube-volume-permission
- jkube-persistentvolumeclaim-storageclass
- jkube-configmap-file
- jkube-secret-file

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
# TODO: Documents and verify enrichers below
- jkube-debug
- jkube-volume-permission
- jkube-persistentvolumeclaim-storageclass
- jkube-configmap-file
- jkube-secret-file

Expand Down
5 changes: 5 additions & 0 deletions jkube-kit/doc/src/main/asciidoc/inc/_enricher.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ include::enricher/project/_jkube_openshift_project_entry.adoc[]
include::enricher/route/_jkube_openshift_route_entry.adoc[]
endif::[]

|<<jkube-persistentvolumeclaim-storageclass>>
| Add name of StorageClass required by PersistentVolumeClaim either in metadata or in spec.

| <<jkube-pod-annotation>>
| Copy over annotations from a `Deployment` to a `Pod`

Expand Down Expand Up @@ -200,6 +203,8 @@ include::enricher/port-name/_jkube_port_name.adoc[]

include::enricher/project-label/_jkube_project_label.adoc[]

include::enricher/persistentvolumeclaim-storageclass/_jkube_persistentvolumeclaim_storageclass.adoc[]

include::enricher/_jkube_replicas.adoc[]

include::enricher/revisionhistory/_jkube_revision_history.adoc[]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[[jkube-persistentvolumeclaim-storageclass]]
==== jkube-persistentvolumeclaim-storageclass

Enricher which fixes adds name of StorageClass required by PersistentVolumeClaim either in metadata or in spec.

.Supported properties
[cols="1,6,1"]
|===
| Option | Description | Property

| *defaultStorageClass*
| PersistentVolume storage class.
| `jkube.enricher.jkube-volume-permission.defaultStorageClass`

| *useStorageClassAnnotation*
| If enabled, storage class would be added to PersistentVolumeClaim metadata as `volume.beta.kubernetes.io/storage-class=<storageClassName>` annotation rather than `.spec.storageClassName`

Defaults to `false`
| `jkube.enricher.jkube-volume-permission.useStorageClassAnnotation`
|===
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,15 @@ Defaults to `777`.
| `jkube.enricher.jkube-volume-permission.permission`

| *defaultStorageClass*
| PersistentVolume storage class.
| _Deprecated: Use <<jkube-persistentvolumeclaim-storageclass, PersistentVolumeClaimStorageClassEnricher>>'s defaultStorageClass field_

PersistentVolume storage class.
| `jkube.enricher.jkube-volume-permission.defaultStorageClass`

| *useStorageClassAnnotation*
| If enabled, storage class would be added to PersistentVolumeClaim metadata as `volume.beta.kubernetes.io/storage-class=<storageClassName>` annotation rather than `.spec.storageClassName`
| _Deprecated: Use <<jkube-persistentvolumeclaim-storageclass, PersistentVolumeClaimStorageClassEnricher>>'s defaultStorageClass field_

If enabled, storage class would be added to PersistentVolumeClaim metadata as `volume.beta.kubernetes.io/storage-class=<storageClassName>` annotation rather than `.spec.storageClassName`

Defaults to `false`
| `jkube.enricher.jkube-volume-permission.useStorageClassAnnotation`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* Copyright (c) 2019 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 org.eclipse.jkube.enricher.generic;

import io.fabric8.kubernetes.api.builder.TypedVisitor;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimBuilder;
import lombok.AllArgsConstructor;
import lombok.Getter;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.jkube.kit.common.Configs;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.enricher.api.BaseEnricher;
import org.eclipse.jkube.kit.enricher.api.JKubeEnricherContext;

public class PersistentVolumeClaimStorageClassEnricher extends BaseEnricher {

public static final String ENRICHER_NAME = "jkube-persistentvolumeclaim-storageclass";
static final String VOLUME_STORAGE_CLASS_ANNOTATION = "volume.beta.kubernetes.io/storage-class";

@AllArgsConstructor
enum Config implements Configs.Config {
DEFAULT_STORAGE_CLASS("defaultStorageClass", null),
USE_ANNOTATION("useStorageClassAnnotation", "false");

@Getter
protected String key;
@Getter
protected String defaultValue;
}

public PersistentVolumeClaimStorageClassEnricher(JKubeEnricherContext buildContext) {
super(buildContext, ENRICHER_NAME);
}

@Override
public void enrich(PlatformMode platformMode, KubernetesListBuilder builder) {
builder.accept(new TypedVisitor<PersistentVolumeClaimBuilder>() {
@Override
public void visit(PersistentVolumeClaimBuilder pvcBuilder) {
// lets ensure we have a default storage class so that PVs will get dynamically created OOTB
if (pvcBuilder.buildMetadata() == null) {
pvcBuilder.withNewMetadata().endMetadata();
}
String storageClass = getStorageClass();
if (StringUtils.isNotBlank(storageClass)) {
if (shouldUseAnnotation()) {
pvcBuilder.editMetadata().addToAnnotations(VOLUME_STORAGE_CLASS_ANNOTATION, storageClass).endMetadata();
} else {
pvcBuilder.editSpec().withStorageClassName(storageClass).endSpec();
}
}
}
});
}

private boolean shouldUseAnnotation() {
if (Boolean.TRUE.equals(Boolean.parseBoolean(getConfig(Config.USE_ANNOTATION)))) {
return true;
}
VolumePermissionEnricher volumePermissionEnricher = new VolumePermissionEnricher((JKubeEnricherContext) getContext());
return Boolean.TRUE.equals(volumePermissionEnricher.shouldUseAnnotation());
}

private String getStorageClass() {
String storageClassConfig = getConfig(Config.DEFAULT_STORAGE_CLASS);
if (StringUtils.isNotBlank(storageClassConfig)) {
return storageClassConfig;
}
VolumePermissionEnricher volumePermissionEnricher = new VolumePermissionEnricher((JKubeEnricherContext) getContext());
return volumePermissionEnricher.getDefaultStorageClass();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimBuilder;
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimVolumeSource;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.PodTemplateSpecBuilder;
Expand Down Expand Up @@ -51,13 +50,20 @@
public class VolumePermissionEnricher extends BaseEnricher {

public static final String ENRICHER_NAME = "jkube-volume-permission";
static final String VOLUME_STORAGE_CLASS_ANNOTATION = "volume.beta.kubernetes.io/storage-class";

@AllArgsConstructor
enum Config implements Configs.Config {
IMAGE_NAME("imageName", "busybox"),
PERMISSION("permission", "777"),
/**
* @deprecated Use configuration field in PersistentVolumeClaimStorageClassEnricher
*/
@Deprecated
DEFAULT_STORAGE_CLASS("defaultStorageClass", null),
/**
* @deprecated Use configuration field in PersistentVolumeClaimStorageClassEnricher
*/
@Deprecated
USE_ANNOTATION("useStorageClassAnnotation", "false"),
CPU_LIMIT("cpuLimit", null),
CPU_REQUEST("cpuRequest", null),
Expand Down Expand Up @@ -199,23 +205,27 @@ private Map<String, Quantity> createResourcesMap(Configs.Config cpu, Configs.Con
return resourcesMap;
}
});
}

builder.accept(new TypedVisitor<PersistentVolumeClaimBuilder>() {
@Override
public void visit(PersistentVolumeClaimBuilder pvcBuilder) {
// lets ensure we have a default storage class so that PVs will get dynamically created OOTB
if (pvcBuilder.buildMetadata() == null) {
pvcBuilder.withNewMetadata().endMetadata();
}
String storageClass = getConfig(Config.DEFAULT_STORAGE_CLASS);
if (StringUtils.isNotBlank(storageClass)) {
if (Boolean.parseBoolean(getConfig(Config.USE_ANNOTATION))) {
pvcBuilder.editMetadata().addToAnnotations(VOLUME_STORAGE_CLASS_ANNOTATION, storageClass).endMetadata();
} else {
pvcBuilder.editSpec().withStorageClassName(storageClass).endSpec();
}
}
}
});
/**
* Get useAnnotation from enricher configuration
* TODO: This method is kept only for backward compatibility. This should
* be removed in future. See <a href="https://github.com/eclipse/jkube/issues/1989">GitHub Issue</a> for more details
*
* @return boolean value indicating whether StorageClass annotation should be used or not
*/
public boolean shouldUseAnnotation() {
return Boolean.parseBoolean(getConfig(Config.USE_ANNOTATION));
}

/**
* Get Default StorageClass from enricher configuration
*
* TODO: This method is kept only for backward compatibility. This should
* be removed in future. See <a href="https://github.com/eclipse/jkube/issues/1989">GitHub Issue</a> for more details
* @return default storage class
*/
public String getDefaultStorageClass() {
return getConfig(Config.DEFAULT_STORAGE_CLASS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,6 @@ org.eclipse.jkube.enricher.generic.ReplicaCountEnricher

# Set ImagePullPolicy on a Controller's container
org.eclipse.jkube.enricher.generic.ImagePullPolicyEnricher

# Set StorageClass name in PersistentVolumeClaim annotation or in spec
org.eclipse.jkube.enricher.generic.PersistentVolumeClaimStorageClassEnricher
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright (c) 2019 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 org.eclipse.jkube.enricher.generic;

import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.PersistentVolumeClaim;
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimBuilder;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.enricher.api.JKubeEnricherContext;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.util.Collections;
import java.util.Properties;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class PersistentVolumeClaimStorageClassEnricherTest {
private JKubeEnricherContext context;

@BeforeEach
void setUp() {
context = mock(JKubeEnricherContext.class, RETURNS_DEEP_STUBS);
}

@ParameterizedTest
@ValueSource(strings = {"jkube-persistentvolumeclaim-storageclass", "jkube-volume-permission"})
void enrich_withPersistentVolumeClaim_shouldAddStorageClassToSpec(String enricher) {
// Given
Properties properties = new Properties();
properties.put("jkube.enricher." + enricher + ".defaultStorageClass", "standard");
when(context.getProperties()).thenReturn(properties);
PersistentVolumeClaimStorageClassEnricher volumePermissionEnricher = new PersistentVolumeClaimStorageClassEnricher(context);
KubernetesListBuilder klb = new KubernetesListBuilder();
klb.addToItems(createNewPersistentVolumeClaim());

// When
volumePermissionEnricher.enrich(PlatformMode.kubernetes, klb);

// Then
assertThat(klb.buildItems())
.singleElement(InstanceOfAssertFactories.type(PersistentVolumeClaim.class))
.hasFieldOrPropertyWithValue("spec.storageClassName", "standard")
.extracting("metadata.annotations")
.asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class))
.isNullOrEmpty();
}

@ParameterizedTest
@ValueSource(strings = {"jkube-persistentvolumeclaim-storageclass", "jkube-volume-permission"})
void enrich_withPersistentVolumeClaimAndUseAnnotationEnabled_shouldAddStorageClassAnnotation(String enricher) {
// Given
Properties properties = new Properties();
properties.put("jkube.enricher." + enricher + ".defaultStorageClass", "standard");
properties.put("jkube.enricher." + enricher + ".useStorageClassAnnotation", "true");
when(context.getProperties()).thenReturn(properties);
PersistentVolumeClaimStorageClassEnricher volumePermissionEnricher = new PersistentVolumeClaimStorageClassEnricher(context);
KubernetesListBuilder klb = new KubernetesListBuilder();
klb.addToItems(createNewPersistentVolumeClaim());

// When
volumePermissionEnricher.enrich(PlatformMode.kubernetes, klb);

// Then
assertThat(klb.buildItems())
.singleElement(InstanceOfAssertFactories.type(PersistentVolumeClaim.class))
.hasFieldOrPropertyWithValue("metadata.annotations", Collections.singletonMap("volume.beta.kubernetes.io/storage-class", "standard"))
.hasFieldOrPropertyWithValue("spec.storageClassName", null);
}

private PersistentVolumeClaim createNewPersistentVolumeClaim() {
return new PersistentVolumeClaimBuilder()
.withNewMetadata().withName("pv1").endMetadata()
.withNewSpec().endSpec()
.build();
}
}
Loading

0 comments on commit 63b4cf5

Please sign in to comment.