From 60df737b749212328d566b20a8882c6baec7ae23 Mon Sep 17 00:00:00 2001 From: Anil Kedia Date: Wed, 14 Jul 2021 16:26:08 +0000 Subject: [PATCH 1/5] OWLS-90857 - Create copy of init container env variables for each server to avoid the roll issue. --- .../kubernetes/operator/helpers/PodHelper.java | 2 +- .../weblogic/domain/model/ServerPod.java | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java index cef1e14a1eb..414d0ac50f2 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java @@ -259,7 +259,7 @@ public static Step deletePodStep(String serverName, Step next) { return new DeletePodStep(serverName, next); } - static List createCopy(List envVars) { + public static List createCopy(List envVars) { ArrayList copy = new ArrayList<>(); if (envVars != null) { for (V1EnvVar envVar : envVars) { diff --git a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java index 0ac0e6af298..21ef76405e8 100644 --- a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java +++ b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java @@ -43,6 +43,7 @@ import org.apache.commons.lang3.builder.ToStringBuilder; import static java.util.Collections.emptyList; +import static oracle.kubernetes.operator.helpers.PodHelper.createCopy; class ServerPod extends KubernetesResource { @@ -443,7 +444,7 @@ void fillInFrom(ServerPod serverPod1) { addIfMissing(var); } for (V1Container c : serverPod1.getInitContainers()) { - addInitContainerIfMissing(c); + addInitContainerIfMissing(copyContainer(c)); } for (V1Container c : serverPod1.getContainers()) { addContainerIfMissing(c); @@ -492,6 +493,17 @@ private void addAuxiliaryImage(ServerPod serverPod1) { } } + private V1Container copyContainer(V1Container c) { + return new V1Container().args(c.getArgs()).command(c.getCommand()).env(createCopy(c.getEnv())) + .envFrom(c.getEnvFrom()).image(c.getImage()).imagePullPolicy(c.getImagePullPolicy()) + .lifecycle(c.getLifecycle()).livenessProbe(c.getLivenessProbe()).name(c.getName()).ports(c.getPorts()) + .readinessProbe(c.getReadinessProbe()).resources(c.getResources()).securityContext(c.getSecurityContext()) + .startupProbe(c.getStartupProbe()).stdin(c.getStdin()).stdinOnce(c.getStdinOnce()) + .terminationMessagePath(c.getTerminationMessagePath()) + .terminationMessagePolicy(c.getTerminationMessagePolicy()).tty(c.getTty()) + .volumeDevices(c.getVolumeDevices()).volumeMounts(c.getVolumeMounts()).workingDir(c.getWorkingDir()); + } + private void addIfMissing(V1Volume var) { if (!hasVolumeName(var.getName())) { addAdditionalVolume(var); From 9d0c4a1c3a6bcdf286c206be360a993dd546148a Mon Sep 17 00:00:00 2001 From: Anil Kedia Date: Wed, 14 Jul 2021 19:00:49 +0000 Subject: [PATCH 2/5] Added debug and javadoc. --- .../java/oracle/kubernetes/operator/helpers/PodHelper.java | 6 ++++++ .../oracle/kubernetes/operator/helpers/PodStepContext.java | 7 ++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java index 414d0ac50f2..af4664685fc 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java @@ -259,6 +259,12 @@ public static Step deletePodStep(String serverName, Step next) { return new DeletePodStep(serverName, next); } + /** + * Copy list of V1EnvVar environment variables. + * + * @param envVars list of environment variables to copy + * @return List containing a copy of the original list. + */ public static List createCopy(List envVars) { ArrayList copy = new ArrayList<>(); if (envVars != null) { diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java index b0b7e1a73f9..71f22a114f7 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java @@ -37,6 +37,7 @@ import io.kubernetes.client.openapi.models.V1SecretVolumeSource; import io.kubernetes.client.openapi.models.V1Volume; import io.kubernetes.client.openapi.models.V1VolumeMount; +import io.kubernetes.client.util.Yaml; import jakarta.json.Json; import jakarta.json.JsonPatchBuilder; import oracle.kubernetes.operator.DomainSourceType; @@ -545,11 +546,11 @@ private boolean canUseCurrentPod(V1Pod currentPod) { boolean useCurrent = hasCorrectPodHash(currentPod) && canUseNewDomainZip(currentPod); - if (!useCurrent && AnnotationHelper.getDebugString(currentPod).length() > 0) { + if (!useCurrent) { LOGGER.fine( MessageKeys.POD_DUMP, - AnnotationHelper.getDebugString(currentPod), - AnnotationHelper.getDebugString(getPodModel())); + Yaml.dump(currentPod), + Yaml.dump(getPodModel())); } return useCurrent; From 3c520c59525cd2c01f93850a7c5cebffdc7c5e01 Mon Sep 17 00:00:00 2001 From: Anil Kedia Date: Thu, 15 Jul 2021 15:34:45 +0000 Subject: [PATCH 3/5] Fix java doc and rename method to clarify the intent. --- .../oracle/kubernetes/operator/helpers/PodHelper.java | 2 +- .../kubernetes/weblogic/domain/model/ServerPod.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java index af4664685fc..7c7ce8ee301 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java @@ -260,7 +260,7 @@ public static Step deletePodStep(String serverName, Step next) { } /** - * Copy list of V1EnvVar environment variables. + * Create a copy of the list of V1EnvVar environment variables. * * @param envVars list of environment variables to copy * @return List containing a copy of the original list. diff --git a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java index 21ef76405e8..5856609f43e 100644 --- a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java +++ b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java @@ -38,12 +38,12 @@ import jakarta.validation.Valid; import oracle.kubernetes.json.Description; import oracle.kubernetes.json.Feature; +import oracle.kubernetes.operator.helpers.PodHelper; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; import static java.util.Collections.emptyList; -import static oracle.kubernetes.operator.helpers.PodHelper.createCopy; class ServerPod extends KubernetesResource { @@ -444,7 +444,7 @@ void fillInFrom(ServerPod serverPod1) { addIfMissing(var); } for (V1Container c : serverPod1.getInitContainers()) { - addInitContainerIfMissing(copyContainer(c)); + addInitContainerIfMissing(createWithEnvCopy(c)); } for (V1Container c : serverPod1.getContainers()) { addContainerIfMissing(c); @@ -493,8 +493,8 @@ private void addAuxiliaryImage(ServerPod serverPod1) { } } - private V1Container copyContainer(V1Container c) { - return new V1Container().args(c.getArgs()).command(c.getCommand()).env(createCopy(c.getEnv())) + private V1Container createWithEnvCopy(V1Container c) { + return new V1Container().args(c.getArgs()).command(c.getCommand()).env(PodHelper.createCopy(c.getEnv())) .envFrom(c.getEnvFrom()).image(c.getImage()).imagePullPolicy(c.getImagePullPolicy()) .lifecycle(c.getLifecycle()).livenessProbe(c.getLivenessProbe()).name(c.getName()).ports(c.getPorts()) .readinessProbe(c.getReadinessProbe()).resources(c.getResources()).securityContext(c.getSecurityContext()) From 4210f2d015cd13868c65e7311c4d1671674b2c06 Mon Sep 17 00:00:00 2001 From: Anil Kedia Date: Fri, 16 Jul 2021 15:19:52 +0000 Subject: [PATCH 4/5] Changed the log level for pod dump message in case of roll and message for loading scripts in config map. --- .../oracle/kubernetes/operator/helpers/ConfigMapHelper.java | 2 +- .../java/oracle/kubernetes/operator/helpers/PodStepContext.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/ConfigMapHelper.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/ConfigMapHelper.java index c48199b0d68..cfd84534285 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/ConfigMapHelper.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/ConfigMapHelper.java @@ -203,7 +203,7 @@ void recordCurrentMap(Packet packet, V1ConfigMap configMap) { static synchronized Map loadScriptsFromClasspath(String domainNamespace) { Map scripts = scriptReader.loadFilesFromClasspath(); - LOGGER.fine(MessageKeys.SCRIPT_LOADED, domainNamespace); + LOGGER.finer(MessageKeys.SCRIPT_LOADED, domainNamespace); return scripts; } diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java index 71f22a114f7..753c9257ed4 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java @@ -547,7 +547,7 @@ private boolean canUseCurrentPod(V1Pod currentPod) { boolean useCurrent = hasCorrectPodHash(currentPod) && canUseNewDomainZip(currentPod); if (!useCurrent) { - LOGGER.fine( + LOGGER.finer( MessageKeys.POD_DUMP, Yaml.dump(currentPod), Yaml.dump(getPodModel())); From c1cda3c590f19831e39845c0a441fc2a0d280004 Mon Sep 17 00:00:00 2001 From: Anil Kedia Date: Fri, 16 Jul 2021 21:41:32 +0000 Subject: [PATCH 5/5] Changes as per PR review comments. --- .../kubernetes/operator/helpers/PodHelper.java | 6 ++---- .../kubernetes/weblogic/domain/model/ServerPod.java | 12 +++--------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java index 7c7ce8ee301..c293a130628 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java @@ -13,6 +13,7 @@ import io.kubernetes.client.openapi.models.V1DeleteOptions; import io.kubernetes.client.openapi.models.V1EnvVar; +import io.kubernetes.client.openapi.models.V1EnvVarBuilder; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Pod; import io.kubernetes.client.openapi.models.V1PodCondition; @@ -272,10 +273,7 @@ public static List createCopy(List envVars) { // note that a deep copy of valueFrom is not needed here as, unlike with value, the // new V1EnvVarFrom objects would be created by the doDeepSubstitutions() method in // StepContextBase class. - copy.add(new V1EnvVar() - .name(envVar.getName()) - .value(envVar.getValue()) - .valueFrom(envVar.getValueFrom())); + copy.add(new V1EnvVarBuilder(envVar).build()); } } return copy; diff --git a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java index 5856609f43e..bff58b4d51c 100644 --- a/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java +++ b/operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerPod.java @@ -17,6 +17,7 @@ import io.kubernetes.client.openapi.models.V1Affinity; import io.kubernetes.client.openapi.models.V1Capabilities; import io.kubernetes.client.openapi.models.V1Container; +import io.kubernetes.client.openapi.models.V1ContainerBuilder; import io.kubernetes.client.openapi.models.V1EnvVar; import io.kubernetes.client.openapi.models.V1HostPathVolumeSource; import io.kubernetes.client.openapi.models.V1NodeAffinity; @@ -38,12 +39,12 @@ import jakarta.validation.Valid; import oracle.kubernetes.json.Description; import oracle.kubernetes.json.Feature; -import oracle.kubernetes.operator.helpers.PodHelper; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.builder.ToStringBuilder; import static java.util.Collections.emptyList; +import static oracle.kubernetes.operator.helpers.PodHelper.createCopy; class ServerPod extends KubernetesResource { @@ -494,14 +495,7 @@ private void addAuxiliaryImage(ServerPod serverPod1) { } private V1Container createWithEnvCopy(V1Container c) { - return new V1Container().args(c.getArgs()).command(c.getCommand()).env(PodHelper.createCopy(c.getEnv())) - .envFrom(c.getEnvFrom()).image(c.getImage()).imagePullPolicy(c.getImagePullPolicy()) - .lifecycle(c.getLifecycle()).livenessProbe(c.getLivenessProbe()).name(c.getName()).ports(c.getPorts()) - .readinessProbe(c.getReadinessProbe()).resources(c.getResources()).securityContext(c.getSecurityContext()) - .startupProbe(c.getStartupProbe()).stdin(c.getStdin()).stdinOnce(c.getStdinOnce()) - .terminationMessagePath(c.getTerminationMessagePath()) - .terminationMessagePolicy(c.getTerminationMessagePolicy()).tty(c.getTty()) - .volumeDevices(c.getVolumeDevices()).volumeMounts(c.getVolumeMounts()).workingDir(c.getWorkingDir()); + return new V1ContainerBuilder(c).withEnv(createCopy(c.getEnv())).build(); } private void addIfMissing(V1Volume var) {