This is an automated email from the ASF dual-hosted git repository.
gyfora pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/flink-kubernetes-operator.git
The following commit(s) were added to refs/heads/main by this push:
new ddada0e8 [FLINK-31246] Beautify the SpecChange message
ddada0e8 is described below
commit ddada0e835a6078be70f0828434d5fada7f335b1
Author: pvary <[email protected]>
AuthorDate: Sun Mar 5 15:30:19 2023 +0100
[FLINK-31246] Beautify the SpecChange message
---
.../AbstractFlinkResourceReconciler.java | 2 +-
.../operator/reconciler/diff/DiffBuilder.java | 14 ++--
.../operator/reconciler/diff/DiffResult.java | 88 ++++++++++++++++++----
.../reconciler/diff/ReflectiveDiffBuilder.java | 24 +++---
.../operator/reconciler/diff/SpecDiffTest.java | 34 +++++++++
5 files changed, 127 insertions(+), 35 deletions(-)
diff --git
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractFlinkResourceReconciler.java
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractFlinkResourceReconciler.java
index 2098c07c..c74545c9 100644
---
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractFlinkResourceReconciler.java
+++
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractFlinkResourceReconciler.java
@@ -127,7 +127,7 @@ public abstract class AbstractFlinkResourceReconciler<
cr.getStatus().getReconciliationStatus().deserializeLastReconciledSpec();
SPEC currentDeploySpec = cr.getSpec();
- var specDiff = new ReflectiveDiffBuilder<>(currentDeploySpec,
lastReconciledSpec).build();
+ var specDiff = new ReflectiveDiffBuilder<>(lastReconciledSpec,
currentDeploySpec).build();
boolean specChanged =
DiffType.IGNORE != specDiff.getType()
diff --git
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffBuilder.java
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffBuilder.java
index a58615de..7c27eb88 100644
---
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffBuilder.java
+++
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffBuilder.java
@@ -40,17 +40,17 @@ public class DiffBuilder<T> implements
Builder<DiffResult<?>> {
private static final String DELIMITER = ".";
- private final T left;
- private final T right;
+ private final T before;
+ private final T after;
private final List<Diff<?>> diffs;
private boolean triviallyEqual;
- public DiffBuilder(@NonNull final T left, @NonNull final T right) {
+ public DiffBuilder(@NonNull final T before, @NonNull final T after) {
this.diffs = new ArrayList<>();
- this.left = left;
- this.right = right;
- this.triviallyEqual = left == right || left.equals(right);
+ this.before = before;
+ this.after = after;
+ this.triviallyEqual = before == after || before.equals(after);
}
public DiffBuilder<T> testTriviallyEqual(boolean testTriviallyEqual) {
@@ -348,6 +348,6 @@ public class DiffBuilder<T> implements
Builder<DiffResult<?>> {
@Override
public DiffResult<T> build() {
- return new DiffResult<>(left, right, diffs);
+ return new DiffResult<>(before, after, diffs);
}
}
diff --git
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffResult.java
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffResult.java
index 7ada6d01..d53921b1 100644
---
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffResult.java
+++
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/DiffResult.java
@@ -21,10 +21,11 @@ import org.apache.flink.annotation.Experimental;
import org.apache.flink.kubernetes.operator.api.diff.DiffType;
import org.apache.flink.kubernetes.operator.api.diff.Diffable;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.fabric8.zjsonpatch.JsonDiff;
import lombok.Getter;
import lombok.NonNull;
-import org.apache.commons.lang3.builder.ToStringBuilder;
-import org.apache.commons.lang3.builder.ToStringStyle;
import java.util.List;
@@ -38,13 +39,15 @@ import java.util.List;
@Getter
public class DiffResult<T> {
@NonNull private final List<Diff<?>> diffList;
- @NonNull private final T left;
- @NonNull private final T right;
+ @NonNull private final T before;
+ @NonNull private final T after;
@NonNull private final DiffType type;
- DiffResult(@NonNull T left, @NonNull T right, @NonNull List<Diff<?>>
diffList) {
- this.left = left;
- this.right = right;
+ private static final ObjectMapper objectMapper = new ObjectMapper();
+
+ DiffResult(@NonNull T before, @NonNull T after, @NonNull List<Diff<?>>
diffList) {
+ this.before = before;
+ this.after = after;
this.diffList = diffList;
this.type = getSpechChangeType(diffList);
}
@@ -59,18 +62,42 @@ public class DiffResult<T> {
return "";
}
- final ToStringBuilder lhsBuilder =
- new ToStringBuilder(left, ToStringStyle.SHORT_PREFIX_STYLE);
- final ToStringBuilder rhsBuilder =
- new ToStringBuilder(right, ToStringStyle.SHORT_PREFIX_STYLE);
+ final StringBuilder builder = new StringBuilder();
+ builder.append(before.getClass().getSimpleName()).append("[");
diffList.forEach(
diff -> {
- lhsBuilder.append(diff.getFieldName(), diff.getLeft());
- rhsBuilder.append(diff.getFieldName(), diff.getRight());
+ try {
+ JsonNode diffBefore =
+ objectMapper.readTree(
+
objectMapper.writeValueAsString(diff.getLeft()));
+ JsonNode diffAfter =
+ objectMapper.readTree(
+
objectMapper.writeValueAsString(diff.getRight()));
+ JsonNode jsonDiff = JsonDiff.asJson(diffBefore,
diffAfter);
+ jsonDiff.forEach(
+ row -> {
+ addField(
+ builder,
+ diffBefore,
+ diffAfter,
+ diff.getFieldName(),
+ row);
+ builder.append(", ");
+ });
+ builder.setLength(builder.length() - 2);
+ } catch (Exception je) {
+ builder.append(diff.getFieldName())
+ .append(" : ")
+ .append(diff.getLeft())
+ .append(" -> ")
+ .append(diff.getRight());
+ }
+ builder.append(", ");
});
-
- return String.format("%s differs from %s", lhsBuilder.build(),
rhsBuilder.build());
+ builder.setLength(builder.length() - 2);
+ builder.append("]");
+ return String.format("Diff: %s", builder);
}
private static DiffType getSpechChangeType(List<Diff<?>> diffs) {
@@ -83,4 +110,35 @@ public class DiffResult<T> {
}
return type;
}
+
+ private static void addField(
+ StringBuilder sb,
+ JsonNode parentBefore,
+ JsonNode parentAfter,
+ String fieldName,
+ JsonNode diff) {
+ JsonNode beforeNode = parentBefore;
+ JsonNode afterNode = parentAfter;
+ String extraPath = "";
+ if (!diff.get("path").asText().equals("/")) {
+ extraPath = diff.get("path").asText().replaceAll("/", ".");
+ beforeNode = beforeNode.at(diff.get("path").asText());
+ afterNode = afterNode.at(diff.get("path").asText());
+ }
+ sb.append(fieldName).append(extraPath).append(" : ");
+ if ((afterNode.isNull() || afterNode.isMissingNode()) &&
beforeNode.asText().equals("")) {
+ sb.append("{..}");
+ } else {
+ sb.append(getText(beforeNode));
+ }
+ sb.append(" -> ").append(getText(afterNode));
+ }
+
+ private static String getText(JsonNode node) {
+ if (node.isNull() || node.isMissingNode()) {
+ return null;
+ }
+ String text = node.asText();
+ return text.equals("") ? node.toString() : text;
+ }
}
diff --git
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/ReflectiveDiffBuilder.java
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/ReflectiveDiffBuilder.java
index 73cbdc65..2bbf618b 100644
---
a/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/ReflectiveDiffBuilder.java
+++
b/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/diff/ReflectiveDiffBuilder.java
@@ -46,23 +46,23 @@ import static
org.apache.flink.kubernetes.operator.api.diff.DiffType.UPGRADE;
@Experimental
public class ReflectiveDiffBuilder<T> implements Builder<DiffResult<T>> {
- private final Object left;
- private final Object right;
+ private final Object before;
+ private final Object after;
private final DiffBuilder<T> diffBuilder;
- public ReflectiveDiffBuilder(@NonNull final T lhs, @NonNull final T rhs) {
- this.left = lhs;
- this.right = rhs;
- diffBuilder = new DiffBuilder<>(lhs, rhs);
+ public ReflectiveDiffBuilder(@NonNull final T before, @NonNull final T
after) {
+ this.before = before;
+ this.after = after;
+ diffBuilder = new DiffBuilder<>(before, after);
}
@Override
public DiffResult<T> build() {
- if (left.equals(right)) {
+ if (before.equals(after)) {
return diffBuilder.build();
}
- appendFields(left.getClass());
+ appendFields(before.getClass());
return diffBuilder.build();
}
@@ -70,8 +70,8 @@ public class ReflectiveDiffBuilder<T> implements
Builder<DiffResult<T>> {
for (final Field field : FieldUtils.getAllFields(clazz)) {
if (accept(field)) {
try {
- var leftField = readField(field, left, true);
- var rightField = readField(field, right, true);
+ var leftField = readField(field, before, true);
+ var rightField = readField(field, after, true);
if (field.isAnnotationPresent(SpecDiff.Config.class)
&& Map.class.isAssignableFrom(field.getType())) {
diffBuilder.append(
@@ -99,8 +99,8 @@ public class ReflectiveDiffBuilder<T> implements
Builder<DiffResult<T>> {
} else {
diffBuilder.append(
field.getName(),
- readField(field, left, true),
- readField(field, right, true),
+ readField(field, before, true),
+ readField(field, after, true),
UPGRADE);
}
diff --git
a/flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/reconciler/diff/SpecDiffTest.java
b/flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/reconciler/diff/SpecDiffTest.java
index 30c75e62..3f36970c 100644
---
a/flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/reconciler/diff/SpecDiffTest.java
+++
b/flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/reconciler/diff/SpecDiffTest.java
@@ -30,6 +30,7 @@ import
org.apache.flink.kubernetes.operator.api.utils.BaseTestUtils;
import org.apache.flink.kubernetes.operator.api.utils.SpecUtils;
import
org.apache.flink.kubernetes.operator.config.KubernetesOperatorConfigOptions;
+import io.fabric8.kubernetes.api.model.HostAlias;
import org.junit.jupiter.api.Test;
import java.util.List;
@@ -181,4 +182,37 @@ public class SpecDiffTest {
assertEquals(DiffType.UPGRADE, diff.getType());
assertEquals(11, diff.getNumDiffs());
}
+
+ @Test
+ public void testPodTemplateChanges() {
+ var left = BaseTestUtils.buildApplicationCluster().getSpec();
+ left.setPodTemplate(BaseTestUtils.getTestPod("localhost1", "v1",
List.of()));
+ left.getPodTemplate()
+ .getSpec()
+ .getHostAliases()
+ .add(new HostAlias(List.of("host1", "host2"), "ip"));
+ left.setImage("img1");
+ IngressSpec ingressSpec = new IngressSpec();
+ ingressSpec.setTemplate("temp");
+ left.setIngress(ingressSpec);
+ var right = BaseTestUtils.buildApplicationCluster().getSpec();
+ right.setPodTemplate(BaseTestUtils.getTestPod("localhost2", "v2",
List.of()));
+ right.getPodTemplate()
+ .getSpec()
+ .getHostAliases()
+ .add(new HostAlias(List.of("host1"), "ip"));
+ right.setImage("img2");
+ right.setRestartNonce(1L);
+
+ var diff = new ReflectiveDiffBuilder<>(left, right).build();
+ assertEquals(DiffType.UPGRADE, diff.getType());
+ assertEquals(
+ "Diff: FlinkDeploymentSpec[image : img1 -> img2, "
+ + "ingress : {..} -> null, "
+ + "podTemplate.apiVersion : v1 -> v2, "
+ + "podTemplate.spec.hostAliases.0.hostnames.1 : host2
-> null, "
+ + "podTemplate.spec.hostname : localhost1 ->
localhost2, "
+ + "restartNonce : null -> 1]",
+ diff.toString());
+ }
}
