Skip to content

feat: dependent resource in the condition instead of resource #1690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions docs/documentation/v4-3-migration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
title: Migrating from v4.2 to v4.3
description: Migrating from v4.2 to v4.3
layout: docs
permalink: /docs/v4-3-migration
---

# Migrating from v4.2 to v4.3

## Condition API Change

In Workflows the target of the condition was the managed resource itself, not a dependent resource. This changed, from
not the API contains the dependent resource.

New API:

```java
public interface Condition<R, P extends HasMetadata> {

boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);

}
```

Former API:

```java
public interface Condition<R, P extends HasMetadata> {

boolean isMet(P primary, R secondary, Context<P> context);

}
```

Migration is trivial. Since the secondary resource can be accessed from the dependent resource. So to access the secondary
resource just use `dependentResource.getSecondaryResource(primary,context)`.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource;

@SuppressWarnings("rawtypes")
public abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
Expand Down Expand Up @@ -102,15 +101,7 @@ protected synchronized void handleNodeExecutionFinish(

protected <R> boolean isConditionMet(Optional<Condition<R, P>> condition,
DependentResource<R, P> dependentResource) {
if (condition.isEmpty()) {
return true;
}
var resources = dependentResource instanceof BulkDependentResource
? ((BulkDependentResource) dependentResource).getSecondaryResources(primary, context)
: dependentResource.getSecondaryResource(primary, context).orElse(null);

return condition.map(c -> c.isMet(primary,
(R) resources, context))
return condition.map(c -> c.isMet(dependentResource, primary, context))
.orElse(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;

public interface Condition<R, P extends HasMetadata> {

Expand All @@ -10,12 +11,10 @@ public interface Condition<R, P extends HasMetadata> {
* {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the
* observed cluster state.
*
* @param dependentResource for which the condition applies to
* @param primary the primary resource being considered
* @param secondary the secondary resource associated with the specified primary resource or
* {@code null} if no such secondary resource exists (for example because it's been
* deleted)
* @param context the current reconciliation {@link Context}
* @return {@code true} if the condition holds, {@code false} otherwise
*/
boolean isMet(P primary, R secondary, Context<P> context);
boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe keep the order that was there before (i.e. primary as first param)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, don't, have a strong opinion on that. Changed this because the condition applies primarily to a dependent resource, so that's why it is on first place (kinda leading or priority)

}
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,9 @@ public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap>
private static class TestCondition implements Condition<ConfigMap, ConfigMap> {

@Override
public boolean isMet(ConfigMap primary, ConfigMap secondary, Context<ConfigMap> context) {
public boolean isMet(DependentResource<ConfigMap, ConfigMap> dependentResource,
ConfigMap primary,
Context<ConfigMap> context) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
package io.javaoperatorsdk.operator.sample.bulkdependent;

import java.util.Map;

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class SampleBulkCondition
implements Condition<Map<String, ConfigMap>, BulkDependentTestCustomResource> {
implements Condition<ConfigMap, BulkDependentTestCustomResource> {

// We use ConfigMaps here just to show how to check some properties of resources managed by a
// BulkDependentResource. In real life example this would be rather based on some status of those
// resources, like Pods.

@Override
public boolean isMet(BulkDependentTestCustomResource primary, Map<String, ConfigMap> secondary,
public boolean isMet(
DependentResource<ConfigMap, BulkDependentTestCustomResource> dependentResource,
BulkDependentTestCustomResource primary,
Context<BulkDependentTestCustomResource> context) {

return secondary.values().stream().allMatch(cm -> !cm.getData().isEmpty());

var resources = ((CRUDConfigMapBulkDependentResource) dependentResource)
.getSecondaryResources(primary, context);
return resources.values().stream().noneMatch(cm -> cm.getData().isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,23 @@

import io.fabric8.kubernetes.api.model.apps.StatefulSet;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
import io.javaoperatorsdk.operator.sample.complexdependent.ComplexDependentCustomResource;

public class StatefulSetReadyCondition
implements Condition<StatefulSet, ComplexDependentCustomResource> {

@Override
public boolean isMet(ComplexDependentCustomResource primary, StatefulSet secondary,
public boolean isMet(
DependentResource<StatefulSet, ComplexDependentCustomResource> dependentResource,
ComplexDependentCustomResource primary,
Context<ComplexDependentCustomResource> context) {

var readyReplicas = secondary.getStatus().getReadyReplicas();
return readyReplicas != null && readyReplicas > 0;
return dependentResource.getSecondaryResource(primary, context).map(secondary -> {
var readyReplicas = secondary.getStatus().getReadyReplicas();
return readyReplicas != null && readyReplicas > 0;
})
.orElse(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class ConfigMapDeletePostCondition
Expand All @@ -14,9 +15,11 @@ public class ConfigMapDeletePostCondition

@Override
public boolean isMet(
WorkflowAllFeatureCustomResource primary, ConfigMap secondary,
DependentResource<ConfigMap, WorkflowAllFeatureCustomResource> dependentResource,
WorkflowAllFeatureCustomResource primary,
Context<WorkflowAllFeatureCustomResource> context) {
var configMapDeleted = secondary == null;

var configMapDeleted = dependentResource.getSecondaryResource(primary, context).isEmpty();
log.debug("Config Map Deleted: {}", configMapDeleted);
return configMapDeleted;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

import io.fabric8.kubernetes.api.model.ConfigMap;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class ConfigMapReconcileCondition
implements Condition<ConfigMap, WorkflowAllFeatureCustomResource> {

@Override
public boolean isMet(WorkflowAllFeatureCustomResource primary, ConfigMap secondary,
public boolean isMet(
DependentResource<ConfigMap, WorkflowAllFeatureCustomResource> dependentResource,
WorkflowAllFeatureCustomResource primary,
Context<WorkflowAllFeatureCustomResource> context) {

return primary.getSpec().isCreateConfigMap();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@

import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class DeploymentReadyCondition
implements Condition<Deployment, WorkflowAllFeatureCustomResource> {
@Override
public boolean isMet(WorkflowAllFeatureCustomResource primary, Deployment deployment,
public boolean isMet(
DependentResource<Deployment, WorkflowAllFeatureCustomResource> dependentResource,
WorkflowAllFeatureCustomResource primary,
Context<WorkflowAllFeatureCustomResource> context) {
if (deployment == null) {
return false;
}
var readyReplicas = deployment.getStatus().getReadyReplicas();

return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas);
return dependentResource.getSecondaryResource(primary, context)
.map(deployment -> {
var readyReplicas = deployment.getStatus().getReadyReplicas();
return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas);
})
.orElse(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import io.fabric8.kubernetes.api.model.networking.v1.Ingress;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;

public class ExposedIngressCondition implements Condition<Ingress, WebPage> {

@Override
public boolean isMet(WebPage primary, Ingress secondary, Context<WebPage> context) {
public boolean isMet(DependentResource<Ingress, WebPage> dependentResource,
WebPage primary, Context<WebPage> context) {
return primary.getSpec().getExposed();
}
}