Skip to content

Commit fe5c951

Browse files
csviricsviri
committed
Integration tests
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
1 parent d51a2e3 commit fe5c951

9 files changed

Lines changed: 635 additions & 11 deletions

File tree

‎docs/content/en/blog/news/read-after-write-consistency.md‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> contex
3030
}
3131
```
3232

33-
In addition to that, the framework will automatically filter events for your own updates,
34-
so they don't trigger the reconciliation again.
33+
In addition to that, the framework will provide facilities to filter out
34+
events for own updates so they don't trigger the reconciliation again.
3535

3636
{{% alert color=success %}}
3737
**This should significantly simplify controller development, and will make reconciliation
@@ -180,12 +180,6 @@ From this point the idea of the algorithm is very simple:
180180
the one in the TRC. If yes, evict the resource from the TRC.
181181
3. When the controller reads a resource from cache, it checks the TRC first, then falls back to the Informer's cache.
182182

183-
The actual filtering of events for our own writes is more nuanced than a simple
184-
"evict on RV ≥ TRC version" rule — it is driven by a per-resource state machine
185-
that tracks in-flight writes and the events received around them. See
186-
[Filtering events for our own updates](#filtering-events-for-our-own-updates) below.
187-
188-
189183
```mermaid
190184
sequenceDiagram
191185
box rgba(50,108,229,0.1)
@@ -226,10 +220,30 @@ sequenceDiagram
226220
When we update a resource, the informer will eventually propagate an event that would trigger a reconciliation.
227221
In most cases, however, this is not desirable. Since we already have the up-to-date resource at that point,
228222
we want to be notified only when the change originates outside our reconciler.
229-
Therefore, in addition to caching the resource, we filter out events caused by our own updates.
223+
Therefore, in addition to caching the resource, provide utilities to so you can optimize the update/patch
224+
operations to filter out those events.
225+
226+
See [ResourceOperations](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java#L49)
227+
for details.
228+
229+
```java
230+
public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> context) {
231+
232+
ConfigMap managedConfigMap = prepareConfigMap(webPage);
233+
234+
// resource operation in this case will resource only if
235+
// it does not match the actual, and will filter our the related event
236+
context.resourceOperations().serverSideApply(managedConfigMap);
237+
238+
// UpdateControl.patchStatus would only cache the resource to filter out events too
239+
// you have to use resourceOperations.
240+
context.resourceOperations().serverSideApplyPrimaryStatus(alterStatusObject(webPage));
241+
return UpdateControl.noUpdate();
242+
}
243+
```
230244

231-
Note that the implementation of this is relatively complex: while performing the update, we record all the
232-
events received in the meantime and decide whether to propagate them further once the update request completes.
245+
Note that the implementation of this is relatively complex and has some caveats:
246+
while performing the update, we record all the events received in the meantime and decide whether to propagate them further once the update request completes.
233247

234248
This way, we significantly reduce the number of reconciliations, making the whole process much more efficient.
235249

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/matcher/PatchMatchersTest.java‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.api.reconciler.matcher;
17+
18+
import org.junit.jupiter.api.Test;
19+
20+
import io.fabric8.kubernetes.api.model.HasMetadata;
21+
import io.fabric8.kubernetes.api.model.apps.Deployment;
22+
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
23+
import io.fabric8.kubernetes.client.KubernetesClient;
24+
import io.javaoperatorsdk.operator.MockKubernetesClient;
25+
import io.javaoperatorsdk.operator.api.reconciler.Context;
26+
import io.javaoperatorsdk.operator.api.reconciler.DefaultContext;
27+
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
import static org.mockito.Mockito.mock;
30+
31+
class PatchMatchersTest {
32+
33+
private static final Context<?> context = new TestContext();
34+
35+
private final JsonPatchMacher jsonPatchMatcher = JsonPatchMacher.getInstance();
36+
private final JsonMergePatchMatcher mergePatchMatcher = JsonMergePatchMatcher.getInstance();
37+
private final JsonPatchStatusMacher jsonPatchStatusMatcher = JsonPatchStatusMacher.getInstance();
38+
private final JsonMergePatchStatusMatcher mergePatchStatusMatcher =
39+
JsonMergePatchStatusMatcher.getInstance();
40+
41+
// ---- JSON Patch (whole resource) ----
42+
43+
@Test
44+
void jsonPatchMatchesIdenticalResources() {
45+
assertThat(jsonPatchMatcher.matches(deployment(), deployment(), context)).isTrue();
46+
}
47+
48+
@Test
49+
void jsonPatchDoesNotMatchWhenActualHasAdditionalField() {
50+
var actual = deployment();
51+
actual.getMetadata().getLabels().put("extra", "value");
52+
assertThat(jsonPatchMatcher.matches(deployment(), actual, context)).isFalse();
53+
}
54+
55+
@Test
56+
void jsonPatchDoesNotMatchWhenDesiredHasAdditionalField() {
57+
var desired = deployment();
58+
desired.getMetadata().getLabels().put("extra", "value");
59+
assertThat(jsonPatchMatcher.matches(desired, deployment(), context)).isFalse();
60+
}
61+
62+
@Test
63+
void jsonPatchDoesNotMatchOnDifferentValue() {
64+
var desired = deployment();
65+
desired.getSpec().setReplicas(5);
66+
assertThat(jsonPatchMatcher.matches(desired, deployment(), context)).isFalse();
67+
}
68+
69+
// ---- JSON Merge Patch (whole resource) ----
70+
71+
@Test
72+
void mergePatchMatchesIdenticalResources() {
73+
assertThat(mergePatchMatcher.matches(deployment(), deployment(), context)).isTrue();
74+
}
75+
76+
@Test
77+
void mergePatchMatchesWhenActualHasAdditionalField() {
78+
// a merge patch of the desired state leaves fields only present in the actual state untouched
79+
var actual = deployment();
80+
actual.getMetadata().getLabels().put("extra", "value");
81+
assertThat(mergePatchMatcher.matches(deployment(), actual, context)).isTrue();
82+
}
83+
84+
@Test
85+
void mergePatchDoesNotMatchWhenDesiredHasAdditionalField() {
86+
var desired = deployment();
87+
desired.getMetadata().getLabels().put("extra", "value");
88+
assertThat(mergePatchMatcher.matches(desired, deployment(), context)).isFalse();
89+
}
90+
91+
@Test
92+
void mergePatchDoesNotMatchOnDifferentValue() {
93+
var desired = deployment();
94+
desired.getSpec().setReplicas(5);
95+
assertThat(mergePatchMatcher.matches(desired, deployment(), context)).isFalse();
96+
}
97+
98+
// ---- Status matchers ----
99+
100+
@Test
101+
void statusMatchersIgnoreChangesOutsideStatus() {
102+
var desired = deploymentWithStatus();
103+
desired.getSpec().setReplicas(5);
104+
var actual = deploymentWithStatus();
105+
assertThat(jsonPatchStatusMatcher.matches(desired, actual, context)).isTrue();
106+
assertThat(mergePatchStatusMatcher.matches(desired, actual, context)).isTrue();
107+
}
108+
109+
@Test
110+
void statusMatchersDoNotMatchOnDifferentStatus() {
111+
var desired = deploymentWithStatus();
112+
desired.getStatus().setReplicas(9);
113+
var actual = deploymentWithStatus();
114+
assertThat(jsonPatchStatusMatcher.matches(desired, actual, context)).isFalse();
115+
assertThat(mergePatchStatusMatcher.matches(desired, actual, context)).isFalse();
116+
}
117+
118+
@Test
119+
void statusMatchersDifferOnAdditionalActualStatusField() {
120+
var actual = deploymentWithStatus();
121+
actual.getStatus().setAvailableReplicas(1);
122+
var desired = deploymentWithStatus();
123+
124+
// json patch sees the extra actual field as a removal, so it does not match
125+
assertThat(jsonPatchStatusMatcher.matches(desired, actual, context)).isFalse();
126+
// merge patch tolerates fields only present in the actual state
127+
assertThat(mergePatchStatusMatcher.matches(desired, actual, context)).isTrue();
128+
}
129+
130+
@Test
131+
void statusMatchersMatchWhenBothStatusesAbsent() {
132+
assertThat(jsonPatchStatusMatcher.matches(deployment(), deployment(), context)).isTrue();
133+
assertThat(mergePatchStatusMatcher.matches(deployment(), deployment(), context)).isTrue();
134+
}
135+
136+
private static Deployment deployment() {
137+
return new DeploymentBuilder()
138+
.withNewMetadata()
139+
.withName("test")
140+
.withNamespace("default")
141+
.addToLabels("app", "test")
142+
.endMetadata()
143+
.withNewSpec()
144+
.withReplicas(1)
145+
.endSpec()
146+
.build();
147+
}
148+
149+
private static Deployment deploymentWithStatus() {
150+
var deployment = deployment();
151+
deployment.setStatus(new io.fabric8.kubernetes.api.model.apps.DeploymentStatus());
152+
deployment.getStatus().setReplicas(1);
153+
return deployment;
154+
}
155+
156+
private static class TestContext extends DefaultContext<HasMetadata> {
157+
private final KubernetesClient client = MockKubernetesClient.client(HasMetadata.class);
158+
159+
TestContext() {
160+
super(mock(), mock(), null, false, false);
161+
}
162+
163+
@Override
164+
public KubernetesClient getClient() {
165+
return client;
166+
}
167+
}
168+
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/matcher/StatusMatchersTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.api.reconciler.matcher;
17+
18+
import org.junit.jupiter.api.Test;
19+
20+
import io.fabric8.kubernetes.api.model.HasMetadata;
21+
import io.fabric8.kubernetes.api.model.apps.Deployment;
22+
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
23+
import io.fabric8.kubernetes.api.model.apps.DeploymentStatusBuilder;
24+
import io.fabric8.kubernetes.client.KubernetesClient;
25+
import io.javaoperatorsdk.operator.MockKubernetesClient;
26+
import io.javaoperatorsdk.operator.api.reconciler.Context;
27+
import io.javaoperatorsdk.operator.api.reconciler.DefaultContext;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
import static org.mockito.Mockito.mock;
31+
32+
class StatusMatchersTest {
33+
34+
private static final Context<?> context = new TestContext();
35+
36+
private final UpdateStatusMatcher updateStatusMatcher = UpdateStatusMatcher.getInstance();
37+
private final SSAStatusMatcher ssaStatusMatcher = SSAStatusMatcher.getInstance();
38+
39+
@Test
40+
void matchesEqualStatus() {
41+
var desired = deploymentWithStatus(1);
42+
var actual = deploymentWithStatus(1);
43+
assertThat(updateStatusMatcher.matches(desired, actual, context)).isTrue();
44+
assertThat(ssaStatusMatcher.matches(desired, actual, context)).isTrue();
45+
}
46+
47+
@Test
48+
void doesNotMatchDifferentStatus() {
49+
var desired = deploymentWithStatus(1);
50+
var actual = deploymentWithStatus(2);
51+
assertThat(updateStatusMatcher.matches(desired, actual, context)).isFalse();
52+
assertThat(ssaStatusMatcher.matches(desired, actual, context)).isFalse();
53+
}
54+
55+
@Test
56+
void ignoresChangesOutsideStatus() {
57+
var desired = deploymentWithStatus(1);
58+
desired.getSpec().setReplicas(5);
59+
var actual = deploymentWithStatus(1);
60+
assertThat(updateStatusMatcher.matches(desired, actual, context)).isTrue();
61+
assertThat(ssaStatusMatcher.matches(desired, actual, context)).isTrue();
62+
}
63+
64+
private static Deployment deploymentWithStatus(int statusReplicas) {
65+
return new DeploymentBuilder()
66+
.withNewMetadata()
67+
.withName("test")
68+
.withNamespace("default")
69+
.endMetadata()
70+
.withNewSpec()
71+
.withReplicas(1)
72+
.endSpec()
73+
.withStatus(new DeploymentStatusBuilder().withReplicas(statusReplicas).build())
74+
.build();
75+
}
76+
77+
private static class TestContext extends DefaultContext<HasMetadata> {
78+
private final KubernetesClient client = MockKubernetesClient.client(HasMetadata.class);
79+
80+
TestContext() {
81+
super(mock(), mock(), null, false, false);
82+
}
83+
84+
@Override
85+
public KubernetesClient getClient() {
86+
return client;
87+
}
88+
}
89+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsCustomResource.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.baseapi.resourceoperations;
17+
18+
import io.fabric8.kubernetes.api.model.Namespaced;
19+
import io.fabric8.kubernetes.client.CustomResource;
20+
import io.fabric8.kubernetes.model.annotation.Group;
21+
import io.fabric8.kubernetes.model.annotation.ShortNames;
22+
import io.fabric8.kubernetes.model.annotation.Version;
23+
24+
@Group("sample.javaoperatorsdk")
25+
@Version("v1")
26+
@ShortNames("ropres")
27+
public class ResourceOperationsCustomResource
28+
extends CustomResource<ResourceOperationsSpec, ResourceOperationsStatus>
29+
implements Namespaced {}

0 commit comments

Comments
 (0)