Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit dbf6c2e

Browse files
authoredFeb 16, 2022
feat: generic matcher for Kubernetes dependent resource (#945)
1 parent f205fd4 commit dbf6c2e

File tree

11 files changed

+186
-41
lines changed

11 files changed

+186
-41
lines changed
 

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package io.javaoperatorsdk.operator;
22

3+
import java.io.IOException;
4+
import java.io.InputStream;
35
import java.lang.reflect.InvocationTargetException;
46
import java.lang.reflect.Method;
57
import java.util.Locale;
68

79
import io.fabric8.kubernetes.api.model.HasMetadata;
810
import io.fabric8.kubernetes.api.model.ObjectMeta;
11+
import io.fabric8.kubernetes.client.utils.Serialization;
912
import io.javaoperatorsdk.operator.api.reconciler.Constants;
1013
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
1114
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
@@ -111,7 +114,15 @@ public static Object getSpec(HasMetadata resource) {
111114
Method getSpecMethod = resource.getClass().getMethod("getSpec");
112115
return getSpecMethod.invoke(resource);
113116
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
114-
throw new IllegalStateException(e);
117+
throw new IllegalStateException("No spec found on resource", e);
118+
}
119+
}
120+
121+
public static <T> T loadYaml(Class<T> clazz, Class loader, String yaml) {
122+
try (InputStream is = loader.getResourceAsStream(yaml)) {
123+
return Serialization.unmarshal(is, clazz);
124+
} catch (IOException ex) {
125+
throw new IllegalStateException("Cannot find yaml on classpath: " + yaml);
115126
}
116127
}
117128

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
/** An interface from which to retrieve configuration information. */
1717
public interface ConfigurationService {
1818

19-
Cloner DEFAULT_CLONER = new Cloner() {
20-
private final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
19+
ObjectMapper OBJECT_MAPPER = new ObjectMapper();
2120

21+
Cloner DEFAULT_CLONER = new Cloner() {
2222
@Override
2323
public HasMetadata clone(HasMetadata object) {
2424
try {
@@ -122,4 +122,8 @@ default ExecutorService getExecutorService() {
122122
default boolean closeClientOnStop() {
123123
return true;
124124
}
125+
126+
default ObjectMapper getObjectMapper() {
127+
return OBJECT_MAPPER;
128+
}
125129
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import io.fabric8.kubernetes.api.model.ConfigMap;
4+
import io.fabric8.kubernetes.api.model.HasMetadata;
5+
import io.fabric8.kubernetes.api.model.Secret;
6+
import io.fabric8.zjsonpatch.JsonDiff;
7+
import io.javaoperatorsdk.operator.ReconcilerUtils;
8+
import io.javaoperatorsdk.operator.api.reconciler.Context;
9+
10+
import com.fasterxml.jackson.databind.ObjectMapper;
11+
12+
public class DesiredValueMatcher implements ResourceMatcher {
13+
14+
private final ObjectMapper objectMapper;
15+
16+
public DesiredValueMatcher(ObjectMapper objectMapper) {
17+
this.objectMapper = objectMapper;
18+
}
19+
20+
@Override
21+
public boolean match(HasMetadata actualResource, HasMetadata desiredResource, Context context) {
22+
if (actualResource instanceof Secret) {
23+
return ResourceComparators.compareSecretData((Secret) desiredResource,
24+
(Secret) actualResource);
25+
}
26+
if (actualResource instanceof ConfigMap) {
27+
return ResourceComparators.compareConfigMapData((ConfigMap) desiredResource,
28+
(ConfigMap) actualResource);
29+
}
30+
// reflection will be replaced by this:
31+
// https://github.com/fabric8io/kubernetes-client/issues/3816
32+
var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desiredResource));
33+
var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
34+
var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
35+
for (int i = 0; i < diffJsonPatch.size(); i++) {
36+
String operation = diffJsonPatch.get(i).get("op").asText();
37+
if (!operation.equals("add")) {
38+
return false;
39+
}
40+
}
41+
return true;
42+
}
43+
}

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

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import io.fabric8.kubernetes.api.model.HasMetadata;
1010
import io.fabric8.kubernetes.client.KubernetesClient;
11-
import io.javaoperatorsdk.operator.ReconcilerUtils;
1211
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1312
import io.javaoperatorsdk.operator.api.config.Utils;
1413
import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
@@ -32,6 +31,7 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
3231
protected KubernetesClient client;
3332
private InformerEventSource<R, P> informerEventSource;
3433
private boolean addOwnerReference;
34+
protected ResourceMatcher resourceMatcher;
3535

3636
@Override
3737
public void configureWith(KubernetesDependentResourceConfig config) {
@@ -40,7 +40,7 @@ public void configureWith(KubernetesDependentResourceConfig config) {
4040
}
4141

4242
@SuppressWarnings("unchecked")
43-
private void configureWith(ConfigurationService service, String labelSelector,
43+
private void configureWith(ConfigurationService configService, String labelSelector,
4444
Set<String> namespaces, boolean addOwnerReference) {
4545
final var primaryResourcesRetriever =
4646
(this instanceof PrimaryResourcesRetriever) ? (PrimaryResourcesRetriever<R>) this
@@ -50,26 +50,28 @@ private void configureWith(ConfigurationService service, String labelSelector,
5050
? (AssociatedSecondaryResourceIdentifier<P>) this
5151
: ResourceID::fromResource;
5252
InformerConfiguration<R, P> ic =
53-
InformerConfiguration.from(service, resourceType())
53+
InformerConfiguration.from(configService, resourceType())
5454
.withLabelSelector(labelSelector)
5555
.withNamespaces(namespaces)
5656
.withPrimaryResourcesRetriever(primaryResourcesRetriever)
5757
.withAssociatedSecondaryResourceIdentifier(secondaryResourceIdentifier)
5858
.build();
59-
this.addOwnerReference = addOwnerReference;
60-
informerEventSource = new InformerEventSource<>(ic, client);
59+
configureWith(configService, new InformerEventSource<>(ic, client), addOwnerReference);
6160
}
6261

6362
/**
6463
* Use to share informers between event more resources.
65-
*
64+
*
65+
* @param configurationService get configs
6666
* @param informerEventSource informer to use
6767
* @param addOwnerReference to the created resource
6868
*/
69-
public void configureWith(InformerEventSource<R, P> informerEventSource,
69+
public void configureWith(ConfigurationService configurationService,
70+
InformerEventSource<R, P> informerEventSource,
7071
boolean addOwnerReference) {
7172
this.informerEventSource = informerEventSource;
7273
this.addOwnerReference = addOwnerReference;
74+
initResourceMatcherIfNotSet(configurationService);
7375
}
7476

7577
protected void beforeCreateOrUpdate(R desired, P primary) {
@@ -79,8 +81,8 @@ protected void beforeCreateOrUpdate(R desired, P primary) {
7981
}
8082

8183
@Override
82-
protected boolean match(R actual, R target, Context context) {
83-
return ReconcilerUtils.specsEqual(actual, target);
84+
protected boolean match(R actualResource, R desiredResource, Context context) {
85+
return resourceMatcher.match(actualResource, desiredResource, context);
8486
}
8587

8688
@SuppressWarnings("unchecked")
@@ -107,6 +109,7 @@ protected R update(R actual, R target, P primary, Context context) {
107109

108110
@Override
109111
public Optional<EventSource> eventSource(EventSourceContext<P> context) {
112+
initResourceMatcherIfNotSet(context.getConfigurationService());
110113
if (informerEventSource == null) {
111114
configureWith(context.getConfigurationService(), null, null,
112115
KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT);
@@ -144,4 +147,16 @@ public Optional<R> getResource(P primaryResource) {
144147
public void setKubernetesClient(KubernetesClient kubernetesClient) {
145148
this.client = kubernetesClient;
146149
}
150+
151+
/**
152+
* Override this method to configure resource matcher
153+
*
154+
* @param configurationService config service to mainly access object mapper
155+
*/
156+
protected void initResourceMatcherIfNotSet(ConfigurationService configurationService) {
157+
if (resourceMatcher == null) {
158+
resourceMatcher = new DesiredValueMatcher(configurationService.getObjectMapper());
159+
}
160+
}
161+
147162
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import java.util.Objects;
4+
5+
import io.fabric8.kubernetes.api.model.ConfigMap;
6+
import io.fabric8.kubernetes.api.model.Secret;
7+
8+
public class ResourceComparators {
9+
10+
public static boolean compareConfigMapData(ConfigMap c1, ConfigMap c2) {
11+
return Objects.equals(c1.getData(), c2.getData()) &&
12+
Objects.equals(c1.getBinaryData(), c2.getBinaryData());
13+
}
14+
15+
public static boolean compareSecretData(Secret s1, Secret s2) {
16+
return Objects.equals(s1.getType(), s2.getType()) &&
17+
Objects.equals(s1.getData(), s2.getData()) &&
18+
Objects.equals(s1.getStringData(), s2.getStringData());
19+
}
20+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import io.fabric8.kubernetes.api.model.HasMetadata;
4+
import io.javaoperatorsdk.operator.api.reconciler.Context;
5+
6+
public interface ResourceMatcher {
7+
8+
boolean match(HasMetadata actualResource, HasMetadata desiredResource, Context context);
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import io.fabric8.kubernetes.api.model.apps.Deployment;
6+
import io.javaoperatorsdk.operator.ReconcilerUtils;
7+
8+
import com.fasterxml.jackson.databind.ObjectMapper;
9+
10+
import static org.assertj.core.api.Assertions.assertThat;
11+
12+
class DesiredValueMatcherTest {
13+
14+
DesiredValueMatcher desiredValueMatcher = new DesiredValueMatcher(new ObjectMapper());
15+
16+
@Test
17+
void checksIfDesiredValuesAreTheSame() {
18+
var target1 = createDeployment();
19+
var desired1 = createDeployment();
20+
assertThat(desiredValueMatcher.match(target1, desired1, null)).isTrue();
21+
22+
var target2 = createDeployment();
23+
var desired2 = createDeployment();
24+
target2.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
25+
assertThat(desiredValueMatcher.match(target2, desired2, null))
26+
.withFailMessage("Additive changes should be ok")
27+
.isTrue();
28+
29+
var target3 = createDeployment();
30+
var desired3 = createDeployment();
31+
desired3.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
32+
assertThat(desiredValueMatcher.match(target3, desired3, null))
33+
.withFailMessage("Removed value should not be ok")
34+
.isFalse();
35+
36+
var target4 = createDeployment();
37+
var desired4 = createDeployment();
38+
target4.getSpec().setReplicas(2);
39+
assertThat(desiredValueMatcher.match(target4, desired4, null))
40+
.withFailMessage("Changed values are not ok")
41+
.isFalse();
42+
}
43+
44+
Deployment createDeployment() {
45+
Deployment deployment =
46+
ReconcilerUtils.loadYaml(
47+
Deployment.class, DesiredValueMatcherTest.class, "nginx-deployment.yaml");
48+
return deployment;
49+
}
50+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
apiVersion: apps/v1 # for versions before 1.9.0 use apps/v1beta2
2+
kind: Deployment
3+
metadata:
4+
name: "test"
5+
spec:
6+
progressDeadlineSeconds: 600
7+
revisionHistoryLimit: 10
8+
selector:
9+
matchLabels:
10+
app: "test-dependent"
11+
replicas: 1
12+
template:
13+
metadata:
14+
labels:
15+
app: "test-dependent"
16+
spec:
17+
containers:
18+
- name: nginx
19+
image: nginx:1.17.0
20+
ports:
21+
- containerPort: 80

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import java.io.IOException;
44
import java.io.InputStream;
55
import java.util.List;
6-
import java.util.Objects;
76

87
import io.fabric8.kubernetes.api.model.apps.Deployment;
98
import io.fabric8.kubernetes.client.KubernetesClient;
@@ -72,14 +71,5 @@ protected Deployment desired(StandaloneDependentTestCustomResource primary, Cont
7271
deployment.getMetadata().setNamespace(primary.getMetadata().getNamespace());
7372
return deployment;
7473
}
75-
76-
@Override
77-
protected boolean match(Deployment actual, Deployment target, Context context) {
78-
return Objects.equals(actual.getSpec().getReplicas(), target.getSpec().getReplicas()) &&
79-
actual.getSpec().getTemplate().getSpec().getContainers().get(0).getImage()
80-
.equals(
81-
target.getSpec().getTemplate().getSpec().getContainers().get(0).getImage());
82-
}
83-
8474
}
8575
}

‎sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,4 @@ protected Deployment desired(Tomcat tomcat, Context context) {
4343
.build();
4444
return deployment;
4545
}
46-
47-
@Override
48-
public boolean match(Deployment fetched, Deployment target, Context context) {
49-
return fetched.getSpec().getTemplate().getSpec().getContainers().get(0).getImage()
50-
.equals(target.getSpec().getTemplate().getSpec().getContainers().get(0).getImage());
51-
}
52-
5346
}

‎sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context context) {
6262
WebPageStatus status = new WebPageStatus();
6363

6464
waitUntilConfigMapAvailable(webPage);
65-
status.setHtmlConfigMap(configMapDR.getResource(webPage).get().getMetadata().getName());
65+
status.setHtmlConfigMap(configMapDR.getResource(webPage).orElseThrow().getMetadata().getName());
6666
status.setAreWeGood("Yes!");
6767
status.setErrorMessage(null);
6868
webPage.setStatus(status);
@@ -114,12 +114,6 @@ protected Deployment desired(WebPage webPage, Context context) {
114114
return deployment;
115115
}
116116

117-
@Override
118-
protected boolean match(Deployment actual, Deployment target, Context context) {
119-
// todo comparator
120-
return true;
121-
}
122-
123117
@Override
124118
protected Class<Deployment> resourceType() {
125119
return Deployment.class;
@@ -140,11 +134,6 @@ protected Service desired(WebPage webPage, Context context) {
140134
return service;
141135
}
142136

143-
protected boolean match(Service actual, Service target, Context context) {
144-
// todo comparator
145-
return true;
146-
}
147-
148137
@Override
149138
protected Class<Service> resourceType() {
150139
return Service.class;

0 commit comments

Comments
 (0)
Please sign in to comment.