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

Support of additional volumes in pod #10099

Merged
merged 69 commits into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
da3af5c
Initial design of additionalVolumes support
cthtrifork May 13, 2024
a97329e
Initial work for implementation of additional volumes
KastTrifork Jun 6, 2024
d3fd12b
Added volume mounts to templateContainer, renamed additionalVolumes a…
MichaelMorrisEst Jun 6, 2024
0ca3cc2
Fix some issues found in testing and review comments
MichaelMorrisEst Jun 7, 2024
32f0777
Added implmentation for other components
MichaelMorrisEst Jun 13, 2024
e5782d0
Adding tests
MichaelMorrisEst Jun 18, 2024
09bc97b
update gitignore with systemtest/bin
cthtrifork Jun 27, 2024
5523212
Update gitignore with more systemtest bins
cthtrifork Jun 27, 2024
a506979
Replace CSI with PersistentVolumeClaims
cthtrifork Jun 27, 2024
51a162c
Merge remote-tracking branch 'origin/main' into feature/additionalVol…
cthtrifork Jun 27, 2024
9dfacaf
update docs
cthtrifork Jun 27, 2024
bdfc674
isForbiddenPath
cthtrifork Jun 27, 2024
d7a6944
Merge branch 'strimzi:main' into feature/additionalVolumes
cthtrifork Jun 30, 2024
4a367f3
Fixed test
KastTrifork Jul 1, 2024
f375c3c
make crd_install
cthtrifork Jul 2, 2024
bcc20b8
revert helm-charts/helm3/strimzi-kafka-operator/crds/040-Crd-kafka.ya…
cthtrifork Jul 2, 2024
124d904
revert gitignore changes
cthtrifork Jul 2, 2024
c511396
missing endline
cthtrifork Jul 2, 2024
7d75c9c
checkJsonInclude should only check strimzi api models
cthtrifork Jul 2, 2024
1703c3e
checkJsonPropertyOrder should only be run for io.strimzi.api models
cthtrifork Jul 2, 2024
775e028
move checkJsonPropertyOrder and checkJsonInclude to check all io.stri…
cthtrifork Jul 2, 2024
ba3c1ea
Suggestion to CHANGELOG.md and documentation in con-configuration-poi…
KastTrifork Jul 2, 2024
1b36089
Merge branch 'main' into feature/additionalVolumes
KastTrifork Jul 2, 2024
5544391
revert CrdGeneratorTest
cthtrifork Jul 2, 2024
a13d01a
Update crd-generator/src/main/java/io/strimzi/crdgenerator/CrdGenerat…
cthtrifork Jul 2, 2024
2049a29
Fixed some test
KastTrifork Jul 2, 2024
6815a00
s/pvc/persistentVolumeClaim
cthtrifork Jul 3, 2024
ea2044c
s/getPvc/getPersistentVolumeClaim
cthtrifork Jul 3, 2024
0205e92
withPersistentVolumeClaim
cthtrifork Jul 3, 2024
7588fb4
update packing and documentation with new naming convention for PVCs
cthtrifork Jul 3, 2024
c675025
try withPersistentVolumeClaim() again
cthtrifork Jul 3, 2024
1ee00a4
packing was not updated
cthtrifork Jul 3, 2024
e42e795
Fix method name
pegtrifork Jul 4, 2024
8085b20
Changed mount path for additionalVolume in Test
KastTrifork Jul 4, 2024
ae3de84
wrong path
cthtrifork Jul 4, 2024
6438087
Revert "wrong path"
cthtrifork Jul 4, 2024
059f143
Fix illegal parsing
pegtrifork Jul 4, 2024
7f3daa2
Fix test and remove redundant path check
pegtrifork Jul 4, 2024
edf5624
Update CHANGELOG.md
KastTrifork Jul 5, 2024
17ee14e
Use InvalidResourceException
pegtrifork Jul 5, 2024
83c1825
Update cluster-operator/src/main/java/io/strimzi/operator/cluster/mod…
cthtrifork Jul 7, 2024
b8a8a47
Added validation of volume name and check for duplicates
KastTrifork Jul 10, 2024
187cc66
Updated the documentation based on feedback
KastTrifork Jul 10, 2024
1ccc7c2
Fix test
KastTrifork Jul 11, 2024
663c943
Merge branch 'main' into feature/additionalVolumes
KastTrifork Jul 11, 2024
0496cc2
Added onOf in AdditionalVolume and removed unused import
KastTrifork Jul 11, 2024
a5d30e9
Renaming of variables
KastTrifork Jul 13, 2024
a954dcb
Fixed test after renaming
KastTrifork Jul 15, 2024
011f6f1
CRD install
KastTrifork Jul 16, 2024
7eb8447
removal of duplicate if statement
KastTrifork Jul 16, 2024
d80f6c0
Update CHANGELOG.md
KastTrifork Jul 17, 2024
ccc4171
Update documentation/modules/overview/con-configuration-points-common…
KastTrifork Jul 17, 2024
d5ff6e9
Update documentation/modules/overview/con-configuration-points-common…
KastTrifork Jul 17, 2024
1a58829
Update documentation/modules/overview/con-configuration-points-common…
KastTrifork Jul 17, 2024
d8aefb2
Changes based on feedback, attempt to remove whitespace, refactor che…
KastTrifork Jul 17, 2024
ad46503
Merge branch 'main' into feature/additionalVolumes
KastTrifork Jul 17, 2024
d7e808b
Fixed checkstyle error
KastTrifork Jul 17, 2024
fea2cef
Bug fix
KastTrifork Jul 18, 2024
e1b6a93
Added test for AdditionalVolumes in TemplateUtilsTest
KastTrifork Jul 18, 2024
a3cd0c8
Merge main into branch
KastTrifork Jul 19, 2024
45ed5e1
Fix checkstyle error
pegtrifork Jul 19, 2024
84aa5f7
Finish refactor of check for null
pegtrifork Jul 22, 2024
89933ee
Fix checkstyle
pegtrifork Jul 22, 2024
e009277
Apply suggestions from code review
pegtrifork Jul 23, 2024
ca9eb61
Apply suggestions from code review
pegtrifork Jul 23, 2024
3640dbe
Removed static method imports as per review comment
MichaelMorrisEst Jul 26, 2024
9e0915c
Merge branch 'main' into feature/additionalVolumes
KastTrifork Aug 1, 2024
20199b7
Added variable assertToleration
KastTrifork Aug 1, 2024
c5bb07a
Revert last commit to match main branch
KastTrifork Aug 2, 2024
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## 0.43.0

Added support for additional volumes in CRDs:
* Users can specify volumes and volumeMounts fields in the Kafka, KafkaConnect, KafkaBridge, KafkaMirrorMaker2, EntityOperator, CruiseControl, KafkaExporter, and Zookeeper CRDs.
* Supported volume types include Secret, ConfigMap, EmptyDir, and PersistentVolumeClaims.
* All additional mounted paths is mounted inside /mnt to ensure backwards compatibility for the evolution of kafka and this operator.
KastTrifork marked this conversation as resolved.
Show resolved Hide resolved
* Example configurations and guidelines have been added to the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

Nit, but I think this line is not needed.

## 0.42.0

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright Strimzi authors.
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
*/
package io.strimzi.api.kafka.model.common.template;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import io.fabric8.kubernetes.api.model.ConfigMapVolumeSource;
import io.fabric8.kubernetes.api.model.EmptyDirVolumeSource;
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimVolumeSource;
import io.fabric8.kubernetes.api.model.SecretVolumeSource;
import io.strimzi.api.kafka.model.common.Constants;
import io.strimzi.api.kafka.model.common.UnknownPropertyPreserving;
import io.strimzi.crdgenerator.annotations.Description;
import io.strimzi.crdgenerator.annotations.KubeLink;
import io.sundr.builder.annotations.Buildable;
import lombok.EqualsAndHashCode;
import lombok.ToString;

import java.util.HashMap;
import java.util.Map;

/**
* Representation of additional volumes for Strimzi resources.
*/
@Buildable(editableEnabled = false, builderPackage = Constants.FABRIC8_KUBERNETES_API)
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({ "name", "path", "subPath", "secret", "configMap", "emptyDir", "persistentVolumeClaim" })
@EqualsAndHashCode
@ToString
public class AdditionalVolume implements UnknownPropertyPreserving {
scholzj marked this conversation as resolved.
Show resolved Hide resolved
private String name;
private SecretVolumeSource secret;
private ConfigMapVolumeSource configMap;
private EmptyDirVolumeSource emptyDir;
private PersistentVolumeClaimVolumeSource persistentVolumeClaim;
private Map<String, Object> additionalProperties = new HashMap<>(0);

@Description("Name to use for the volume. Required.")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

@Description("Secret to use populate the volume.")
@KubeLink(group = "core", version = "v1", kind = "secretvolumesource")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public SecretVolumeSource getSecret() {
return secret;
}

public void setSecret(SecretVolumeSource secret) {
this.secret = secret;
}

@Description("ConfigMap to use to populate the volume.")
@KubeLink(group = "core", version = "v1", kind = "configmapvolumesource")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public ConfigMapVolumeSource getConfigMap() {
return configMap;
}

public void setConfigMap(ConfigMapVolumeSource configMap) {
this.configMap = configMap;
}

@Description("EmptyDir to use to populate the volume.")
@KubeLink(group = "core", version = "v1", kind = "emptydirvolumesource")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public EmptyDirVolumeSource getEmptyDir() {
return emptyDir;
}

public void setEmptyDir(EmptyDirVolumeSource emptyDir) {
this.emptyDir = emptyDir;
}

@Description("PersistentVolumeClaim object to use to populate the volume.")
@KubeLink(group = "core", version = "v1", kind = "persistentvolumeclaimvolumesource")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public PersistentVolumeClaimVolumeSource getPersistentVolumeClaim() {
return persistentVolumeClaim;
}

public void setPersistentVolumeClaim(PersistentVolumeClaimVolumeSource persistentVolumeClaim) {
this.persistentVolumeClaim = persistentVolumeClaim;
}

@Override
public Map<String, Object> getAdditionalProperties() {
return this.additionalProperties;
}

@Override
public void setAdditionalProperty(String name, Object value) {
this.additionalProperties.put(name, value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
package io.strimzi.api.kafka.model.common.template;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import io.fabric8.kubernetes.api.model.SecurityContext;
import io.fabric8.kubernetes.api.model.VolumeMount;
import io.strimzi.api.kafka.model.common.Constants;
import io.strimzi.api.kafka.model.common.ContainerEnvVar;
import io.strimzi.api.kafka.model.common.UnknownPropertyPreserving;
Expand All @@ -29,14 +31,25 @@
builderPackage = Constants.FABRIC8_KUBERNETES_API
)
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({"env", "securityContext"})
@JsonPropertyOrder({"env", "securityContext", "volumeMounts"})
@DescriptionFile
@EqualsAndHashCode
@ToString
public class ContainerTemplate implements UnknownPropertyPreserving {
private List<ContainerEnvVar> env;
private SecurityContext securityContext;
private Map<String, Object> additionalProperties;
private List<VolumeMount> additionalVolumeMounts;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why is the field here called additionalVolumeMounts? Why not just call it volumeMounts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have renamed the variable to volumeMounts


@Description("Additional volume mounts which should be applied to the container")
@JsonProperty("volumeMounts")
public List<VolumeMount> getAdditionalVolumeMounts() {
return additionalVolumeMounts;
}

public void setAdditionalVolumeMounts(List<VolumeMount> additionalVolumeMounts) {
this.additionalVolumeMounts = additionalVolumeMounts;
}

@Description("Environment variables which should be applied to the container.")
public List<ContainerEnvVar> getEnv() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
@JsonInclude(JsonInclude.Include.NON_DEFAULT)
@JsonPropertyOrder({"metadata", "imagePullSecrets", "securityContext", "terminationGracePeriodSeconds", "affinity",
"tolerations", "topologySpreadConstraints", "priorityClassName", "schedulerName", "hostAliases",
"enableServiceLinks", "tmpDirSizeLimit"})
"enableServiceLinks", "tmpDirSizeLimit", "volumes"})
@EqualsAndHashCode
@ToString
@DescriptionFile
Expand All @@ -55,6 +55,7 @@ public class PodTemplate implements HasMetadataTemplate, UnknownPropertyPreservi
private List<HostAlias> hostAliases;
private Boolean enableServiceLinks;
private String tmpDirSizeLimit;
private List<AdditionalVolume> additionalVolumes;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why the field is called additionalVolumes and not just volumes? I get that calling the class AdditionalVolume helps to avoid class name conflicts. So that is good and useful. But the field can be simply called just volumes here, or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MichaelMorrisEst What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have renamed the variable to volumes

private Map<String, Object> additionalProperties;

@Description("Metadata applied to the resource.")
Expand Down Expand Up @@ -195,6 +196,17 @@ public void setTmpDirSizeLimit(String tmpDirSizeLimit) {
this.tmpDirSizeLimit = tmpDirSizeLimit;
}

@Description("Additional volumes that can be mounted to the pod.")
@JsonProperty("volumes")
Copy link
Member

Choose a reason for hiding this comment

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

This annotation is not needed anymore I guess?

Copy link
Member

Choose a reason for hiding this comment

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

This should be removed? @JsonProperty("volumes")

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public List<AdditionalVolume> getAdditionalVolumes() {
return additionalVolumes;
}

public void setAdditionalVolumes(List<AdditionalVolume> additionalVolumes) {
this.additionalVolumes = additionalVolumes;
}

@Override
public Map<String, Object> getAdditionalProperties() {
return this.additionalProperties != null ? this.additionalProperties : Map.of();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,23 @@ spec:
runAsUser: 1000001
runAsGroup: 1000001
fsGroup: 0
volumes:
- name: example-secret
secret:
secretName: secret-name
- name: example-configmap
configMap:
name: config-map-name
terminationGracePeriodSeconds: 30
bridgeContainer:
volumeMounts:
- name: example-secret
mountPath: /path/to/mount/secret-volume
subPath: subPath1
initContainer:
volumeMounts:
- name: example-configmap
mountPath: /path/to/mount/cm-volume
podDisruptionBudget:
metadata:
labels:
Expand All @@ -39,4 +55,4 @@ spec:
annotations:
key1: label1
key2: label2
maxUnavailable: 1
maxUnavailable: 1
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,22 @@ spec:
runAsGroup: 1000001
fsGroup: 0
terminationGracePeriodSeconds: 30
volumes:
- name: example-secret
secret:
secretName: secret-name
- name: example-configmap
configMap:
name: config-map-name
connectContainer:
volumeMounts:
- name: example-secret
mountPath: /path/to/mount/secret-volume
subPath: subPath1
initContainer:
volumeMounts:
- name: example-configmap
mountPath: /path/to/mount/cm-volume
podDisruptionBudget:
metadata:
labels:
Expand All @@ -47,4 +63,4 @@ spec:
key2: label2
annotations:
key1: label1
key2: label2
key2: label2
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,28 @@ spec:
runAsGroup: 1000001
fsGroup: 0
terminationGracePeriodSeconds: 30
volumes:
- name: example-secret
secret:
secretName: secret-name
- name: example-configmap
configMap:
name: config-map-name
- name: temp
emptyDir: {}
- name: example-pvc-volume
persistentVolumeClaim:
claimName: myclaim
kafkaContainer:
volumeMounts:
- name: example-secret
mountPath: /mnt/secret-volume
- name: example-configmap
mountPath: /mnt/cm-volume
- name: temp
mountPath: /tmp/logs
- name: example-pvc-volume
mountPath: "/mnt/data"
bootstrapService:
metadata:
labels:
Expand Down Expand Up @@ -120,6 +142,27 @@ spec:
annotations:
key1: label1
key2: label2
volumes:
- name: example-secret
secret:
secretName: secret-name
- name: example-configmap
configMap:
name: config-map-name
- name: temp
emptyDir: {}
- name: example-pvc-volume
persistentVolumeClaim:
claimName: example-pvc-volume
zookeeperContainer:
volumeMounts:
- name: example-secret
mountPath: /mnt/secret-volume
subPath: subPath1
- name: example-configmap
mountPath: /mnt/cm-volume
- name: example-pvc-volume
mountPath: "/mnt/data"
clientService:
metadata:
labels:
Expand Down Expand Up @@ -156,4 +199,16 @@ spec:
annotations:
key1: label1
key2: label2
volumes:
- name: example-secret
secret:
secretName: secret-name
- name: example-configmap
configMap:
name: config-map-name
- name: temp
emptyDir: {}
- name: example-pvc-volume
persistentVolumeClaim:
claimName: example-pvc-volume

Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ spec:
runAsGroup: 1000001
fsGroup: 0
terminationGracePeriodSeconds: 30
volumes:
- name: example-secret
secret:
secretName: secret-name
- name: example-configmap
configMap:
name: config-map-name
connectContainer:
volumeMounts:
- name: example-secret
mountPath: /path/to/mount/secret-volume
subPath: subPath1
initContainer:
volumeMounts:
- name: example-configmap
mountPath: /path/to/mount/cm-volume
podDisruptionBudget:
metadata:
labels:
Expand All @@ -57,4 +73,4 @@ spec:
key2: label2
annotations:
key1: label1
key2: label2
key2: label2
Loading
Loading