Skip to content

Commit 81b756d

Browse files
csvirimetacosm
csviri
and
metacosm
committed
matchers ignore list (#1880)
 Co-authored-by: Chris Laprun <[email protected]>
1 parent f4b5c44 commit 81b756d

File tree

11 files changed

+497
-95
lines changed

11 files changed

+497
-95
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DesiredEqualsMatcher.java

-19
This file was deleted.

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Matcher.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public Optional<T> computedDesired() {
9292
* @return a {@link Result} encapsulating whether the resource matched its desired state and this
9393
* associated state if it was computed as part of the matching process. Use the static
9494
* convenience methods ({@link Result#nonComputed(boolean)} and
95-
* {@link Result#computed(boolean, Object)})
95+
* {@link Result#computed(boolean, Object)}) to create your return {@link Result}.
9696
*/
9797
Result<R> match(R actualResource, P primary, Context<P> context);
9898
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java

+231-60
Large diffs are not rendered by default.

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
142142

143143
@SuppressWarnings("unused")
144144
public Result<R> match(R actualResource, R desired, P primary, Context<P> context) {
145-
return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
145+
return GenericKubernetesResourceMatcher.match(desired, actualResource, false,
146+
false, false);
146147
}
147148

148149
protected void handleDelete(P primary, R secondary, Context<P> context) {

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java

+72-14
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import io.javaoperatorsdk.operator.ReconcilerUtils;
1212
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
1313
import io.javaoperatorsdk.operator.api.reconciler.Context;
14+
import io.javaoperatorsdk.operator.processing.dependent.Matcher;
1415

1516
import static org.assertj.core.api.Assertions.assertThat;
1617
import static org.mockito.Mockito.mock;
@@ -21,44 +22,94 @@ class GenericKubernetesResourceMatcherTest {
2122

2223
private static final Context context = mock(Context.class);
2324

25+
Deployment actual = createDeployment();
26+
Deployment desired = createDeployment();
27+
TestDependentResource dependentResource = new TestDependentResource(desired);
28+
Matcher matcher =
29+
GenericKubernetesResourceMatcher.matcherFor(Deployment.class, dependentResource);
30+
2431
@BeforeAll
2532
static void setUp() {
2633
final var controllerConfiguration = mock(ControllerConfiguration.class);
2734
when(context.getControllerConfiguration()).thenReturn(controllerConfiguration);
2835
}
2936

3037
@Test
31-
void checksIfDesiredValuesAreTheSame() {
32-
var actual = createDeployment();
33-
final var desired = createDeployment();
34-
final var dependentResource = new TestDependentResource(desired);
35-
final var matcher =
36-
GenericKubernetesResourceMatcher.matcherFor(Deployment.class, dependentResource);
38+
void matchesTrivialCases() {
3739
assertThat(matcher.match(actual, null, context).matched()).isTrue();
38-
assertThat(matcher.match(actual, null, context).computedDesired().isPresent()).isTrue();
39-
assertThat(matcher.match(actual, null, context).computedDesired().get()).isEqualTo(desired);
40+
assertThat(matcher.match(actual, null, context).computedDesired()).isPresent();
41+
assertThat(matcher.match(actual, null, context).computedDesired()).contains(desired);
42+
}
4043

44+
@Test
45+
void matchesAdditiveOnlyChanges() {
4146
actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
4247
assertThat(matcher.match(actual, null, context).matched())
43-
.withFailMessage("Additive changes should be ok")
48+
.withFailMessage("Additive changes should not cause a mismatch by default")
4449
.isTrue();
50+
}
4551

52+
@Test
53+
void matchesWithStrongSpecEquality() {
54+
actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
4655
assertThat(GenericKubernetesResourceMatcher
47-
.match(dependentResource, actual, null, context, true, true).matched())
48-
.withFailMessage("Strong equality does not ignore additive changes on spec")
56+
.match(dependentResource, actual, null, context, true, true,
57+
true)
58+
.matched())
59+
.withFailMessage("Adding values should fail matching when strong equality is required")
4960
.isFalse();
61+
}
5062

63+
@Test
64+
void doesNotMatchRemovedValues() {
5165
actual = createDeployment();
5266
assertThat(matcher.match(actual, createPrimary("removed"), context).matched())
53-
.withFailMessage("Removed value should not be ok")
67+
.withFailMessage("Removing values in metadata should lead to a mismatch")
5468
.isFalse();
69+
}
5570

71+
@Test
72+
void doesNotMatchChangedValues() {
5673
actual = createDeployment();
5774
actual.getSpec().setReplicas(2);
5875
assertThat(matcher.match(actual, null, context).matched())
59-
.withFailMessage("Changed values are not ok")
76+
.withFailMessage("Should not have matched because values have changed")
6077
.isFalse();
78+
}
79+
80+
@Test
81+
void doesNotMatchChangedValuesWhenNoIgnoredPathsAreProvided() {
82+
actual = createDeployment();
83+
actual.getSpec().setReplicas(2);
84+
assertThat(GenericKubernetesResourceMatcher
85+
.match(dependentResource, actual, null, context, true).matched())
86+
.withFailMessage(
87+
"Should not have matched because values have changed and no ignored path is provided")
88+
.isFalse();
89+
}
6190

91+
@Test
92+
void doesNotAttemptToMatchIgnoredPaths() {
93+
actual = createDeployment();
94+
actual.getSpec().setReplicas(2);
95+
assertThat(GenericKubernetesResourceMatcher
96+
.match(dependentResource, actual, null, context, false, "/spec/replicas").matched())
97+
.withFailMessage("Should not have compared ignored paths")
98+
.isTrue();
99+
}
100+
101+
@Test
102+
void ignoresWholeSubPath() {
103+
actual = createDeployment();
104+
actual.getSpec().getTemplate().getMetadata().getLabels().put("additional-key", "val");
105+
assertThat(GenericKubernetesResourceMatcher
106+
.match(dependentResource, actual, null, context, false, "/spec/template").matched())
107+
.withFailMessage("Should match when only changes impact ignored sub-paths")
108+
.isTrue();
109+
}
110+
111+
@Test
112+
void matchesMetadata() {
62113
actual = new DeploymentBuilder(createDeployment())
63114
.editOrNewMetadata()
64115
.addToAnnotations("test", "value")
@@ -70,9 +121,16 @@ void checksIfDesiredValuesAreTheSame() {
70121
.isTrue();
71122

72123
assertThat(GenericKubernetesResourceMatcher
73-
.match(dependentResource, actual, null, context, true).matched())
124+
.match(dependentResource, actual, null, context, true, true, true).matched())
74125
.withFailMessage("Annotations should matter when metadata is considered")
75126
.isFalse();
127+
128+
assertThat(GenericKubernetesResourceMatcher
129+
.match(dependentResource, actual, null, context, true, false).matched())
130+
.withFailMessage(
131+
"Should match when strong equality is not considered and only additive changes are made")
132+
.isTrue();
133+
76134
}
77135

78136
Deployment createDeployment() {

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/ServiceStrictMatcherIT.java

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package io.javaoperatorsdk.operator;
2+
3+
import java.time.Duration;
4+
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.extension.RegisterExtension;
7+
8+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
9+
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
10+
import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceDependentResource;
11+
import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceStrictMatcherSpec;
12+
import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceStrictMatcherTestCustomResource;
13+
import io.javaoperatorsdk.operator.sample.servicestrictmatcher.ServiceStrictMatcherTestReconciler;
14+
15+
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.awaitility.Awaitility.await;
17+
18+
public class ServiceStrictMatcherIT {
19+
20+
@RegisterExtension
21+
LocallyRunOperatorExtension operator =
22+
LocallyRunOperatorExtension.builder().withReconciler(new ServiceStrictMatcherTestReconciler())
23+
.build();
24+
25+
26+
@Test
27+
void testTheMatchingDoesNoTTriggersFurtherUpdates() {
28+
var resource = operator.create(testResource());
29+
30+
await().untilAsserted(() -> {
31+
assertThat(operator.getReconcilerOfType(ServiceStrictMatcherTestReconciler.class)
32+
.getNumberOfExecutions()).isEqualTo(1);
33+
});
34+
35+
// make an update to spec to reconcile again
36+
resource.getSpec().setValue(2);
37+
operator.replace(resource);
38+
39+
await().pollDelay(Duration.ofMillis(300)).untilAsserted(() -> {
40+
assertThat(operator.getReconcilerOfType(ServiceStrictMatcherTestReconciler.class)
41+
.getNumberOfExecutions()).isEqualTo(2);
42+
assertThat(ServiceDependentResource.updated.get()).isZero();
43+
});
44+
}
45+
46+
47+
ServiceStrictMatcherTestCustomResource testResource() {
48+
var res = new ServiceStrictMatcherTestCustomResource();
49+
res.setSpec(new ServiceStrictMatcherSpec());
50+
res.getSpec().setValue(1);
51+
res.setMetadata(new ObjectMetaBuilder()
52+
.withName("test1")
53+
.build());
54+
return res;
55+
}
56+
57+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceDependentResource.java

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
2+
3+
import java.util.HashMap;
4+
import java.util.Map;
5+
import java.util.concurrent.atomic.AtomicInteger;
6+
7+
import io.fabric8.kubernetes.api.model.Service;
8+
import io.javaoperatorsdk.operator.ServiceStrictMatcherIT;
9+
import io.javaoperatorsdk.operator.api.reconciler.Context;
10+
import io.javaoperatorsdk.operator.processing.dependent.Matcher;
11+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
12+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher;
13+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
14+
15+
import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml;
16+
17+
@KubernetesDependent
18+
public class ServiceDependentResource
19+
extends CRUDKubernetesDependentResource<Service, ServiceStrictMatcherTestCustomResource> {
20+
21+
public static AtomicInteger updated = new AtomicInteger(0);
22+
23+
public ServiceDependentResource() {
24+
super(Service.class);
25+
}
26+
27+
@Override
28+
protected Service desired(ServiceStrictMatcherTestCustomResource primary,
29+
Context<ServiceStrictMatcherTestCustomResource> context) {
30+
Service service = loadYaml(Service.class, ServiceStrictMatcherIT.class, "service.yaml");
31+
service.getMetadata().setName(primary.getMetadata().getName());
32+
service.getMetadata().setNamespace(primary.getMetadata().getNamespace());
33+
Map<String, String> labels = new HashMap<>();
34+
labels.put("app", "deployment-name");
35+
service.getSpec().setSelector(labels);
36+
return service;
37+
}
38+
39+
@Override
40+
public Matcher.Result<Service> match(Service actualResource,
41+
ServiceStrictMatcherTestCustomResource primary,
42+
Context<ServiceStrictMatcherTestCustomResource> context) {
43+
return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false,
44+
false,
45+
"/spec/ports",
46+
"/spec/clusterIP",
47+
"/spec/clusterIPs",
48+
"/spec/externalTrafficPolicy",
49+
"/spec/internalTrafficPolicy",
50+
"/spec/ipFamilies",
51+
"/spec/ipFamilyPolicy",
52+
"/spec/sessionAffinity");
53+
}
54+
55+
@Override
56+
public Service update(Service actual, Service target,
57+
ServiceStrictMatcherTestCustomResource primary,
58+
Context<ServiceStrictMatcherTestCustomResource> context) {
59+
updated.addAndGet(1);
60+
return super.update(actual, target, primary, context);
61+
}
62+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceStrictMatcherSpec.java

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
2+
3+
public class ServiceStrictMatcherSpec {
4+
5+
private int value;
6+
7+
public int getValue() {
8+
return value;
9+
}
10+
11+
public ServiceStrictMatcherSpec setValue(int value) {
12+
this.value = value;
13+
return this;
14+
}
15+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceStrictMatcherTestCustomResource.java

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
2+
3+
import io.fabric8.kubernetes.api.model.Namespaced;
4+
import io.fabric8.kubernetes.client.CustomResource;
5+
import io.fabric8.kubernetes.model.annotation.Group;
6+
import io.fabric8.kubernetes.model.annotation.ShortNames;
7+
import io.fabric8.kubernetes.model.annotation.Version;
8+
9+
@Group("sample.javaoperatorsdk")
10+
@Version("v1")
11+
@ShortNames("ssm")
12+
public class ServiceStrictMatcherTestCustomResource
13+
extends CustomResource<ServiceStrictMatcherSpec, Void>
14+
implements Namespaced {
15+
16+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceStrictMatcherTestReconciler.java

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package io.javaoperatorsdk.operator.sample.servicestrictmatcher;
2+
3+
import java.util.concurrent.atomic.AtomicInteger;
4+
5+
import io.javaoperatorsdk.operator.api.reconciler.Context;
6+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
7+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
8+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
9+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
10+
11+
@ControllerConfiguration(dependents = {@Dependent(type = ServiceDependentResource.class)})
12+
public class ServiceStrictMatcherTestReconciler
13+
implements Reconciler<ServiceStrictMatcherTestCustomResource> {
14+
15+
16+
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
17+
18+
@Override
19+
public UpdateControl<ServiceStrictMatcherTestCustomResource> reconcile(
20+
ServiceStrictMatcherTestCustomResource resource,
21+
Context<ServiceStrictMatcherTestCustomResource> context) {
22+
numberOfExecutions.addAndGet(1);
23+
return UpdateControl.noUpdate();
24+
}
25+
26+
public int getNumberOfExecutions() {
27+
return numberOfExecutions.get();
28+
}
29+
}

‎operator-framework/src/test/resources/io/javaoperatorsdk/operator/service.yaml

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
apiVersion: v1
2+
kind: Service
3+
metadata:
4+
name: ""
5+
spec:
6+
selector:
7+
app: ""
8+
ports:
9+
- protocol: TCP
10+
port: 80
11+
targetPort: 80
12+
type: NodePort

0 commit comments

Comments
 (0)