Skip to content

Commit c263938

Browse files
russgoldrussgold
committed
improve reporting of pod changes
1 parent f5d813c commit c263938

File tree

2 files changed

+147
-36
lines changed

2 files changed

+147
-36
lines changed

‎operator/src/main/java/oracle/kubernetes/operator/helpers/PodCompatibility.java

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
import static oracle.kubernetes.operator.LabelConstants.DOMAINRESTARTVERSION_LABEL;
99
import static oracle.kubernetes.operator.LabelConstants.SERVERRESTARTVERSION_LABEL;
1010
import static oracle.kubernetes.operator.VersionConstants.DEFAULT_DOMAIN_VERSION;
11+
import static oracle.kubernetes.operator.helpers.PodCompatibility.asSet;
12+
import static oracle.kubernetes.operator.helpers.PodCompatibility.getMissingElements;
1113

1214
import io.kubernetes.client.custom.Quantity;
1315
import io.kubernetes.client.models.V1Container;
@@ -31,6 +33,7 @@
3133
/** A class which defines the compatability rules for existing vs. specified pods. */
3234
class PodCompatibility extends CollectiveCompatibility {
3335
PodCompatibility(V1Pod expected, V1Pod actual) {
36+
add("sha256Hash", AnnotationHelper.getHash(expected), AnnotationHelper.getHash(actual));
3437
add(new PodMetadataCompatibility(expected.getMetadata(), actual.getMetadata()));
3538
add(new PodSpecCompatibility(expected.getSpec(), actual.getSpec()));
3639
}
@@ -101,7 +104,9 @@ static class PodSpecCompatibility extends CollectiveCompatibility {
101104

102105
PodSpecCompatibility(V1PodSpec expected, V1PodSpec actual) {
103106
add("securityContext", expected.getSecurityContext(), actual.getSecurityContext());
104-
add(new EqualsMaps<>("nodeSelector", expected.getNodeSelector(), actual.getNodeSelector()));
107+
add(
108+
new CompatibleMaps<>(
109+
"nodeSelector", expected.getNodeSelector(), actual.getNodeSelector()));
105110
addSets("volumes", expected.getVolumes(), actual.getVolumes());
106111
addSets("imagePullSecrets", expected.getImagePullSecrets(), actual.getImagePullSecrets());
107112
addContainerChecks(expected.getContainers(), actual.getContainers());
@@ -188,28 +193,6 @@ public String getIncompatibility() {
188193
}
189194
}
190195

191-
static class EqualsMaps<K, V> implements CompatibilityCheck {
192-
private final String description;
193-
private final Map<K, V> expected;
194-
private final Map<K, V> actual;
195-
196-
EqualsMaps(String description, Map<K, V> expected, Map<K, V> actual) {
197-
this.description = description;
198-
this.expected = expected;
199-
this.actual = actual;
200-
}
201-
202-
@Override
203-
public boolean isCompatible() {
204-
return KubernetesUtils.mapEquals(expected, actual);
205-
}
206-
207-
@Override
208-
public String getIncompatibility() {
209-
return description + " expected: " + expected + " but was: " + actual;
210-
}
211-
}
212-
213196
static class Probes implements CompatibilityCheck {
214197
private final String description;
215198
private final V1Probe expected;
@@ -283,6 +266,16 @@ public String getIncompatibility() {
283266
return sb.toString();
284267
}
285268
}
269+
270+
static <T> Set<T> asSet(Collection<T> collection) {
271+
return (collection == null) ? Collections.emptySet() : new HashSet<>(collection);
272+
}
273+
274+
static <T> Set<T> getMissingElements(Collection<T> expected, Collection<T> actual) {
275+
Set<T> missing = asSet(expected);
276+
missing.removeAll(actual);
277+
return missing;
278+
}
286279
}
287280

288281
interface CompatibilityCheck {
@@ -318,7 +311,7 @@ public String getIncompatibility() {
318311
}
319312

320313
<T> void addSets(String description, List<T> expected, List<T> actual) {
321-
add(new EqualSets<>(description, expected, actual));
314+
add(new CompatibleSets<>(description, expected, actual));
322315
}
323316

324317
protected <T> void add(String description, T expected, T actual) {
@@ -356,12 +349,12 @@ public String getIncompatibility() {
356349
}
357350
}
358351

359-
class EqualSets<T> implements CompatibilityCheck {
352+
class CompatibleSets<T> implements CompatibilityCheck {
360353
private String description;
361354
private final Collection<T> expected;
362355
private final Collection<T> actual;
363356

364-
EqualSets(String description, Collection<T> expected, Collection<T> actual) {
357+
CompatibleSets(String description, Collection<T> expected, Collection<T> actual) {
365358
this.description = description;
366359
this.expected = expected;
367360
this.actual = actual;
@@ -370,15 +363,50 @@ class EqualSets<T> implements CompatibilityCheck {
370363
@Override
371364
public boolean isCompatible() {
372365
if (expected == actual) return true;
373-
return asSet(expected).equals(asSet(actual));
366+
return asSet(actual).containsAll(asSet(expected));
374367
}
375368

376-
private static <T> Set<T> asSet(Collection<T> collection) {
377-
return (collection == null) ? Collections.emptySet() : new HashSet<>(collection);
369+
@Override
370+
public String getIncompatibility() {
371+
return String.format(
372+
"actual %s does not have %s", description, getMissingElements(expected, actual));
373+
}
374+
}
375+
376+
class CompatibleMaps<K, V> implements CompatibilityCheck {
377+
private final String description;
378+
private final Map<K, V> expected;
379+
private final Map<K, V> actual;
380+
381+
CompatibleMaps(String description, Map<K, V> expected, Map<K, V> actual) {
382+
this.description = description;
383+
this.expected = expected;
384+
this.actual = actual;
385+
}
386+
387+
@Override
388+
public boolean isCompatible() {
389+
for (K key : expected.keySet())
390+
if (!actual.containsKey(key) || !Objects.equals(expected.get(key), actual.get(key)))
391+
return false;
392+
return true;
378393
}
379394

380395
@Override
381396
public String getIncompatibility() {
382-
return String.format("%s expected: %s but was: %s", description, expected, actual);
397+
StringBuilder sb = new StringBuilder();
398+
399+
Set<K> missingKeys = getMissingElements(expected.keySet(), actual.keySet());
400+
if (!missingKeys.isEmpty())
401+
sb.append(String.format("%s has no entry for %s%n", description, missingKeys));
402+
403+
for (K key : expected.keySet())
404+
if (actual.containsKey(key) && !Objects.equals(expected.get(key), actual.get(key)))
405+
sb.append(
406+
String.format(
407+
"%s has entry %s with value %s rather than %s%n",
408+
description, key, actual.get(key), expected.get(key)));
409+
410+
return sb.toString();
383411
}
384412
}

‎operator/src/test/java/oracle/kubernetes/operator/helpers/PodCompatibilityTest.java

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66

77
import static org.hamcrest.Matchers.both;
88
import static org.hamcrest.Matchers.containsString;
9-
import static org.junit.Assert.*;
9+
import static org.hamcrest.Matchers.is;
10+
import static org.hamcrest.Matchers.not;
11+
import static org.junit.Assert.assertThat;
1012

13+
import com.google.common.collect.ImmutableMap;
14+
import com.google.common.collect.ImmutableSet;
1115
import io.kubernetes.client.custom.Quantity;
1216
import io.kubernetes.client.models.V1Container;
1317
import io.kubernetes.client.models.V1ContainerPort;
14-
import io.kubernetes.client.models.V1Pod;
15-
import io.kubernetes.client.models.V1PodSpec;
1618
import io.kubernetes.client.models.V1Probe;
1719
import io.kubernetes.client.models.V1ResourceRequirements;
1820
import org.junit.Test;
@@ -67,10 +69,91 @@ public void whenPortsDontMatch_createErrorMessage() {
6769

6870
assertThat(
6971
compatibility.getIncompatibility(),
70-
both(containsString("1100")).and(containsString("1234")));
72+
both(containsString("1100")).and(not(containsString("1234"))));
7173
}
7274

73-
private V1Pod podWithContainer(V1Container container) {
74-
return new V1Pod().spec(new V1PodSpec().addContainersItem(container));
75+
@Test
76+
public void whenExpectedSubsetOfActual_reportCompatible() {
77+
CompatibilityCheck check =
78+
new CompatibleSets<>("letters", ImmutableSet.of("a", "b"), ImmutableSet.of("b", "c", "a"));
79+
80+
assertThat(check.isCompatible(), is(true));
81+
}
82+
83+
@Test
84+
public void whenExpectedNotSubsetOfActual_reportNotCompatible() {
85+
CompatibilityCheck check =
86+
new CompatibleSets<>(
87+
"letters", ImmutableSet.of("a", "b", "d"), ImmutableSet.of("b", "c", "a"));
88+
89+
assertThat(check.isCompatible(), is(false));
90+
}
91+
92+
@Test
93+
public void whenExpectedNotSubsetOfActual_reportMissingElements() {
94+
CompatibilityCheck check =
95+
new CompatibleSets<>(
96+
"letters",
97+
ImmutableSet.of("alpha", "beta", "delta"),
98+
ImmutableSet.of("beta", "gamma", "alpha"));
99+
100+
assertThat(check.getIncompatibility(), containsString("delta"));
101+
assertThat(check.getIncompatibility(), not(containsString("alpha")));
102+
assertThat(check.getIncompatibility(), not(containsString("gamma")));
103+
}
104+
105+
@Test
106+
public void whenExpectedSubmapOfActual_reportCompatible() {
107+
CompatibilityCheck check =
108+
new CompatibleMaps<>(
109+
"letters", ImmutableMap.of("a", 1, "b", 2), ImmutableMap.of("b", 2, "c", 3, "a", 1));
110+
111+
assertThat(check.isCompatible(), is(true));
112+
}
113+
114+
@Test
115+
public void whenExpectedNotSubmapOfActual_reportNotCompatible() {
116+
CompatibilityCheck check =
117+
new CompatibleMaps<>(
118+
"letters",
119+
ImmutableMap.of("a", 1, "b", 2, "d", 4),
120+
ImmutableMap.of("b", 2, "c", 3, "a", 1));
121+
122+
assertThat(check.isCompatible(), is(false));
123+
}
124+
125+
@Test
126+
public void whenExpectedNotSubmapOfActual_reportMissingElements() {
127+
CompatibilityCheck check =
128+
new CompatibleMaps<>(
129+
"letters",
130+
ImmutableMap.of("alpha", 1, "beta", 2, "delta", 4),
131+
ImmutableMap.of("beta", 2, "gamma", 3, "alpha", 1));
132+
133+
assertThat(check.getIncompatibility(), containsString("delta"));
134+
assertThat(check.getIncompatibility(), not(containsString("alpha")));
135+
assertThat(check.getIncompatibility(), not(containsString("gamma")));
136+
}
137+
138+
@Test
139+
public void whenActualKeysHaveDifferentValues_reportNotCompatible() {
140+
CompatibilityCheck check =
141+
new CompatibleMaps<>(
142+
"letters", ImmutableMap.of("a", 1, "b", 2), ImmutableMap.of("b", 5, "c", 3, "a", 1));
143+
144+
assertThat(check.isCompatible(), is(false));
145+
}
146+
147+
@Test
148+
public void whenActualKeysHaveDifferentValues_reportMissingElements() {
149+
CompatibilityCheck check =
150+
new CompatibleMaps<>(
151+
"letters",
152+
ImmutableMap.of("alpha", 1, "beta", 2),
153+
ImmutableMap.of("beta", 5, "gamma", 3, "alpha", 1));
154+
155+
assertThat(check.getIncompatibility(), both(containsString("beta")).and(containsString("5")));
156+
assertThat(check.getIncompatibility(), not(containsString("alpha")));
157+
assertThat(check.getIncompatibility(), not(containsString("gamma")));
75158
}
76159
}

0 commit comments

Comments
 (0)