Skip to content

fix: incorrect logic by introducing createOrUpdate method #3172

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.

Sign up for GitHub

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
metacosm merged 1 commit into from
Feb 19, 2026
Merged

fix: incorrect logic by introducing createOrUpdate method #3172

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 36 additions & 40 deletions ...operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.BiFunction;

import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -90,55 +90,30 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> contex
return UpdateControl.patchStatus(setInvalidHtmlErrorMessage(webPage));
}

String ns = webPage.getMetadata().getNamespace();
String configMapName = configMapName(webPage);
String deploymentName = deploymentName(webPage);
ConfigMap desiredHtmlConfigMap = makeDesiredHtmlConfigMap(webPage);
Deployment desiredDeployment = makeDesiredDeployment(webPage);
Service desiredService = makeDesiredService(webPage, desiredDeployment);

ConfigMap desiredHtmlConfigMap = makeDesiredHtmlConfigMap(ns, configMapName, webPage);
Deployment desiredDeployment =
makeDesiredDeployment(webPage, deploymentName, ns, configMapName);
Service desiredService = makeDesiredService(webPage, ns, desiredDeployment);

var previousConfigMap = context.getSecondaryResource(ConfigMap.class).orElse(null);
if (!match(desiredHtmlConfigMap, previousConfigMap)) {
log.info(
"Creating or updating ConfigMap {} in {}",
desiredHtmlConfigMap.getMetadata().getName(),
ns);
context.resourceOperations().serverSideApply(desiredHtmlConfigMap);
}

var existingDeployment = context.getSecondaryResource(Deployment.class).orElse(null);
if (!match(desiredDeployment, existingDeployment)) {
log.info(
"Creating or updating Deployment {} in {}",
desiredDeployment.getMetadata().getName(),
ns);
context.resourceOperations().serverSideApply(desiredDeployment);
}

var existingService = context.getSecondaryResource(Service.class).orElse(null);
if (!match(desiredService, existingService)) {
log.info(
"Creating or updating Service {} in {}", desiredDeployment.getMetadata().getName(), ns);
context.resourceOperations().serverSideApply(desiredService);
}
final var previousConfigMap = createOrUpdate(context, desiredHtmlConfigMap, this::match);
createOrUpdate(context, desiredDeployment, this::match);
createOrUpdate(context, desiredService, this::match);

var existingIngress = context.getSecondaryResource(Ingress.class);
if (Boolean.TRUE.equals(webPage.getSpec().getExposed())) {
var desiredIngress = makeDesiredIngress(webPage);
if (existingIngress.isEmpty() || !match(desiredIngress, existingIngress.get())) {
context.resourceOperations().serverSideApply(desiredDeployment);
context.resourceOperations().serverSideApply(desiredIngress);
}
Comment on lines 102 to 106
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The exposed-Ingress reconciliation path was changed (applying desiredIngress instead of desiredDeployment), but there’s no automated coverage for spec.exposed=true in the existing E2E test. Consider extending WebPageOperatorAbstractTest to create a WebPage with spec.setExposed(true) and assert that an Ingress is created/updated; this would prevent regressions of this logic.

Copilot uses AI. Check for mistakes.
} else existingIngress.ifPresent(ingress -> context.getClient().resource(ingress).delete());

// not that this is not necessary, eventually mounted config map would be updated, just this way
// is much faster; what is handy for demo purposes.
// is much faster; this is handy for demo purposes.
// https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/#mounted-configmaps-are-updated-automatically
if (previousConfigMap != null
&& !StringUtils.equals(
&& !Objects.equals(
previousConfigMap.getData().get(INDEX_HTML),
desiredHtmlConfigMap.getData().get(INDEX_HTML))) {
final var ns = webPage.getMetadata().getNamespace();
log.info("Restarting pods because HTML has changed in {}", ns);
context.getClient().pods().inNamespace(ns).withLabel("app", deploymentName(webPage)).delete();
}
Expand All @@ -147,6 +122,21 @@ public UpdateControl<WebPage> reconcile(WebPage webPage, Context<WebPage> contex
createWebPageForStatusUpdate(webPage, desiredHtmlConfigMap.getMetadata().getName()));
}

private <T extends HasMetadata> T createOrUpdate(
Context<WebPage> context, T desired, BiFunction<T, T, Boolean> matcher) {
@SuppressWarnings("unchecked")
final T previous = (T) context.getSecondaryResource(desired.getClass()).orElse(null);
Comment on lines +126 to +128
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

createOrUpdate relies on an unchecked cast from getSecondaryResource(desired.getClass()) to T, requiring @SuppressWarnings("unchecked"). To avoid potential ClassCastException and remove the suppression, consider passing the resource Class<T> explicitly (e.g., createOrUpdate(context, Deployment.class, desiredDeployment, this::match)) or otherwise restructuring so the Optional is typed without a cast.

Copilot uses AI. Check for mistakes.
if (!matcher.apply(desired, previous)) {
log.info(
"Creating or updating {} {} in {}",
desired.getKind(),
desired.getMetadata().getName(),
desired.getMetadata().getNamespace());
context.resourceOperations().serverSideApply(desired);
}
return previous;
Comment on lines +125 to +137
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

CI runs spotless:check with google-java-format; this new createOrUpdate block doesn’t appear to be google-java-format output (long method signature on one line, indentation/alignment inside log.info, extra blank line above). Please run Spotless / apply google-java-format so the file matches the enforced formatter and the PR check passes.

Copilot uses AI. Check for mistakes.
}

private boolean match(Ingress desiredIngress, Ingress existingIngress) {
String desiredServiceName =
desiredIngress
Expand Down Expand Up @@ -205,9 +195,10 @@ private boolean match(ConfigMap desiredHtmlConfigMap, ConfigMap existingConfigMa
}
}

private Service makeDesiredService(WebPage webPage, String ns, Deployment desiredDeployment) {
private Service makeDesiredService(WebPage webPage, Deployment desiredDeployment) {
Service desiredService =
ReconcilerUtilsInternal.loadYaml(Service.class, getClass(), "service.yaml");
final var ns = webPage.getMetadata().getNamespace();
desiredService.getMetadata().setName(serviceName(webPage));
desiredService.getMetadata().setNamespace(ns);
desiredService.getMetadata().setLabels(lowLevelLabel());
Expand All @@ -218,15 +209,18 @@ private Service makeDesiredService(WebPage webPage, String ns, Deployment desire
return desiredService;
}

private Deployment makeDesiredDeployment(
WebPage webPage, String deploymentName, String ns, String configMapName) {
private Deployment makeDesiredDeployment(WebPage webPage) {
Deployment desiredDeployment =
ReconcilerUtilsInternal.loadYaml(Deployment.class, getClass(), "deployment.yaml");
final var ns = webPage.getMetadata().getNamespace();
final var deploymentName = deploymentName(webPage);
desiredDeployment.getMetadata().setName(deploymentName);
desiredDeployment.getMetadata().setNamespace(ns);
desiredDeployment.getMetadata().setLabels(lowLevelLabel());
desiredDeployment.getSpec().getSelector().getMatchLabels().put("app", deploymentName);
desiredDeployment.getSpec().getTemplate().getMetadata().getLabels().put("app", deploymentName);

final var configMapName = configMapName(webPage);
desiredDeployment
.getSpec()
.getTemplate()
Expand All @@ -238,7 +232,9 @@ private Deployment makeDesiredDeployment(
return desiredDeployment;
}

private ConfigMap makeDesiredHtmlConfigMap(String ns, String configMapName, WebPage webPage) {
private ConfigMap makeDesiredHtmlConfigMap(WebPage webPage) {
final var ns = webPage.getMetadata().getNamespace();
final var configMapName = configMapName(webPage);
Map<String, String> data = new HashMap<>();
data.put(INDEX_HTML, webPage.getSpec().getHtml());
ConfigMap configMap =
Expand Down