From 991763f228dd0d77ff84c80d50b92583348e10dd Mon Sep 17 00:00:00 2001 From: "Michael N. Lipp" Date: Sat, 29 Mar 2025 15:09:38 +0100 Subject: [PATCH] Optimize state change handling. --- .../vmoperator/common/VmDefinition.java | 4 +- .../vmoperator/common/VmExtraData.java | 4 +- .../org/jdrupes/vmoperator/common/VmPool.java | 2 +- ...DefChanged.java => VmResourceChanged.java} | 43 ++++--- .../vmoperator/manager/runnerConfig.ftl.yaml | 2 +- .../vmoperator/manager/Controller.java | 6 +- .../manager/DisplaySecretReconciler.java | 12 +- .../manager/LoadBalancerReconciler.java | 10 +- .../vmoperator/manager/PodMonitor.java | 5 +- .../vmoperator/manager/PodReconciler.java | 8 +- .../vmoperator/manager/PoolMonitor.java | 5 +- .../vmoperator/manager/PvcReconciler.java | 59 ++++++---- .../vmoperator/manager/Reconciler.java | 62 ++-------- .../manager/StatefulSetReconciler.java | 107 ------------------ .../jdrupes/vmoperator/manager/VmMonitor.java | 62 +++++----- .../jdrupes/vmoperator/vmaccess/VmAccess.java | 8 +- .../org/jdrupes/vmoperator/vmmgmt/VmMgmt.java | 11 +- webpages/upgrading.md | 4 + 18 files changed, 152 insertions(+), 262 deletions(-) rename org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/{VmDefChanged.java => VmResourceChanged.java} (75%) delete mode 100644 org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/StatefulSetReconciler.java diff --git a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java index f763c47..0a25dd6 100644 --- a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java +++ b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java @@ -300,8 +300,8 @@ public class VmDefinition extends K8sDynamicModel { * * @return the data */ - public Optional extra() { - return Optional.ofNullable(extraData); + public VmExtraData extra() { + return extraData; } /** diff --git a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmExtraData.java b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmExtraData.java index 79f262f..83d9577 100644 --- a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmExtraData.java +++ b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmExtraData.java @@ -112,7 +112,7 @@ public class VmExtraData { * @param deleteConnectionFile the delete connection file * @return the string */ - public String connectionFile(String password, + public Optional connectionFile(String password, Class preferredIpVersion, boolean deleteConnectionFile) { var addr = displayIp(preferredIpVersion); if (addr.isEmpty()) { @@ -144,7 +144,7 @@ public class VmExtraData { if (deleteConnectionFile) { data.append("delete-this-file=1\n"); } - return data.toString(); + return Optional.of(data.toString()); } private Optional displayIp(Class preferredIpVersion) { diff --git a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmPool.java b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmPool.java index 7c13ddb..2bf1c30 100644 --- a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmPool.java +++ b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmPool.java @@ -177,7 +177,7 @@ public class VmPool { } // Additional check in case lastUsed has not been updated - // by PoolMonitor#onVmDefChanged() yet ("race condition") + // by PoolMonitor#onVmResourceChanged() yet ("race condition") if (vmDef.condition("ConsoleConnected") .map(cc -> cc.getLastTransitionTime().toInstant()) .map(this::retainUntil) diff --git a/org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/VmDefChanged.java b/org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/VmResourceChanged.java similarity index 75% rename from org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/VmDefChanged.java rename to org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/VmResourceChanged.java index a8873cf..eac30fb 100644 --- a/org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/VmDefChanged.java +++ b/org.jdrupes.vmoperator.manager.events/src/org/jdrupes/vmoperator/manager/events/VmResourceChanged.java @@ -25,31 +25,35 @@ import org.jgrapes.core.Components; import org.jgrapes.core.Event; /** - * Indicates a change in a VM definition. Note that the definition - * consists of the metadata (mostly immutable), the "spec" and the - * "status" parts. Consumers that are only interested in "spec" - * changes should check {@link #specChanged()} before processing - * the event any further. + * Indicates a change in a VM "resource". Note that the resource + * combines the VM CR's metadata (mostly immutable), the VM CR's + * "spec" part, the VM CR's "status" subresource and state information + * from the pod. Consumers that are only interested in "spec" changes + * should check {@link #specChanged()} before processing the event any + * further. */ @SuppressWarnings("PMD.DataClass") -public class VmDefChanged extends Event { +public class VmResourceChanged extends Event { private final K8sObserver.ResponseType type; - private final boolean specChanged; private final VmDefinition vmDefinition; + private final boolean specChanged; + private final boolean podChanged; /** * Instantiates a new VM changed event. * * @param type the type - * @param specChanged the spec part changed * @param vmDefinition the VM definition + * @param specChanged the spec part changed */ - public VmDefChanged(K8sObserver.ResponseType type, boolean specChanged, - VmDefinition vmDefinition) { + public VmResourceChanged(K8sObserver.ResponseType type, + VmDefinition vmDefinition, boolean specChanged, + boolean podChanged) { this.type = type; - this.specChanged = specChanged; this.vmDefinition = vmDefinition; + this.specChanged = specChanged; + this.podChanged = podChanged; } /** @@ -61,6 +65,15 @@ public class VmDefChanged extends Event { return type; } + /** + * Return the VM definition. + * + * @return the VM definition + */ + public VmDefinition vmDefinition() { + return vmDefinition; + } + /** * Indicates if the "spec" part changed. */ @@ -69,12 +82,10 @@ public class VmDefChanged extends Event { } /** - * Return the VM definition. - * - * @return the VM definition + * Indicates if the pod status changed. */ - public VmDefinition vmDefinition() { - return vmDefinition; + public boolean podChanged() { + return podChanged; } @Override diff --git a/org.jdrupes.vmoperator.manager/resources/org/jdrupes/vmoperator/manager/runnerConfig.ftl.yaml b/org.jdrupes.vmoperator.manager/resources/org/jdrupes/vmoperator/manager/runnerConfig.ftl.yaml index 2a59a2c..5dc8d9b 100644 --- a/org.jdrupes.vmoperator.manager/resources/org/jdrupes/vmoperator/manager/runnerConfig.ftl.yaml +++ b/org.jdrupes.vmoperator.manager/resources/org/jdrupes/vmoperator/manager/runnerConfig.ftl.yaml @@ -53,7 +53,7 @@ data: # i.e. if you start the VM without a value for this property, and # decide to trigger a reset later, you have to first set the value # and then inrement it. - resetCounter: ${ cr.extra().get().resetCount()?c } + resetCounter: ${ cr.extra().resetCount()?c } # Forward the cloud-init data if provided <#if spec.cloudInit??> diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Controller.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Controller.java index ed404c0..eaf2447 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Controller.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Controller.java @@ -1,6 +1,6 @@ /* * VM-Operator - * Copyright (C) 2023 Michael N. Lipp + * Copyright (C) 2023, 2025 Michael N. Lipp * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -46,8 +46,8 @@ import org.jdrupes.vmoperator.manager.events.ModifyVm; import org.jdrupes.vmoperator.manager.events.PodChanged; import org.jdrupes.vmoperator.manager.events.UpdateAssignment; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; import org.jdrupes.vmoperator.manager.events.VmPoolChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jgrapes.core.Channel; import org.jgrapes.core.Component; import org.jgrapes.core.annotation.Handler; @@ -61,7 +61,7 @@ import org.jgrapes.util.events.ConfigurationUpdate; * * The implementation splits the controller in two components. The * {@link VmMonitor} and the {@link Reconciler}. The former watches - * the VM definitions (CRs) and generates {@link VmDefChanged} events + * the VM definitions (CRs) and generates {@link VmResourceChanged} events * when they change. The latter handles the changes and reconciles the * resources in the cluster. * diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java index 2770347..5111438 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/DisplaySecretReconciler.java @@ -45,7 +45,7 @@ import org.jdrupes.vmoperator.common.VmDefinition; import org.jdrupes.vmoperator.common.VmDefinitionStub; import org.jdrupes.vmoperator.manager.events.GetDisplaySecret; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jdrupes.vmoperator.util.DataPath; import org.jgrapes.core.Channel; import org.jgrapes.core.CompletionLock; @@ -123,13 +123,19 @@ public class DisplaySecretReconciler extends Component { * @param vmDef the VM definition * @param model the model * @param channel the channel + * @param specChanged the spec changed * @throws IOException Signals that an I/O exception has occurred. * @throws TemplateException the template exception * @throws ApiException the api exception */ public void reconcile(VmDefinition vmDef, Map model, - VmChannel channel) + VmChannel channel, boolean specChanged) throws IOException, TemplateException, ApiException { + // Nothing to do unless spec changed + if (!specChanged) { + return; + } + // Secret needed at all? var display = vmDef.fromVm("display").get(); if (!DataPath. get(display, "spice", "generateSecret") @@ -292,7 +298,7 @@ public class DisplaySecretReconciler extends Component { */ @Handler @SuppressWarnings("PMD.AvoidSynchronizedStatement") - public void onVmDefChanged(VmDefChanged event, Channel channel) { + public void onVmResourceChanged(VmResourceChanged event, Channel channel) { synchronized (pendingPrepares) { String vmName = event.vmDefinition().name(); for (var pending : pendingPrepares) { diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/LoadBalancerReconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/LoadBalancerReconciler.java index 16af2c8..c0d183b 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/LoadBalancerReconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/LoadBalancerReconciler.java @@ -71,13 +71,19 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; * @param vmDef the VM definition * @param model the model * @param channel the channel + * @param specChanged the spec changed * @throws IOException Signals that an I/O exception has occurred. * @throws TemplateException the template exception * @throws ApiException the api exception */ - public void reconcile(VmDefinition vmDef, - Map model, VmChannel channel) + public void reconcile(VmDefinition vmDef, Map model, + VmChannel channel, boolean specChanged) throws IOException, TemplateException, ApiException { + // Nothing to do unless spec changed + if (!specChanged) { + return; + } + // Check if to be generated @SuppressWarnings({ "PMD.AvoidDuplicateLiterals", "unchecked" }) var lbsDef = Optional.of(model) diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodMonitor.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodMonitor.java index 9145d9d..cfb49e5 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodMonitor.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodMonitor.java @@ -38,7 +38,7 @@ import org.jdrupes.vmoperator.common.K8sV1PodStub; import org.jdrupes.vmoperator.manager.events.ChannelDictionary; import org.jdrupes.vmoperator.manager.events.PodChanged; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jgrapes.core.Channel; import org.jgrapes.core.annotation.Handler; @@ -118,7 +118,8 @@ public class PodMonitor extends AbstractMonitor { * @param channel the channel */ @Handler - public void onVmDefChanged(VmDefChanged event, VmChannel channel) { + public void onVmResourceChanged(VmResourceChanged event, + VmChannel channel) { Optional.ofNullable(pendingChanges.remove(event.vmDefinition().name())) .map(PendingChange::change).ifPresent(change -> { logger.finer(() -> "Firing pending pod change for " diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodReconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodReconciler.java index acda24e..4b7f394 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodReconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PodReconciler.java @@ -64,18 +64,14 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; * @param vmDef the vm def * @param model the model * @param channel the channel + * @param specChanged the spec changed * @throws IOException Signals that an I/O exception has occurred. * @throws TemplateException the template exception * @throws ApiException the api exception */ public void reconcile(VmDefinition vmDef, Map model, - VmChannel channel) + VmChannel channel, boolean specChanged) throws IOException, TemplateException, ApiException { - // Don't do anything if stateful set is still in use (pre v3.4) - if ((Boolean) model.get("usingSts")) { - return; - } - // Get pod stub. var podStub = K8sV1PodStub.get(channel.client(), vmDef.namespace(), vmDef.name()); diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java index 5d85280..1bc323c 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java @@ -40,8 +40,8 @@ import org.jdrupes.vmoperator.common.VmDefinition.Assignment; import org.jdrupes.vmoperator.common.VmDefinitionStub; import org.jdrupes.vmoperator.common.VmPool; import org.jdrupes.vmoperator.manager.events.GetPools; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; import org.jdrupes.vmoperator.manager.events.VmPoolChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jdrupes.vmoperator.util.GsonPtr; import org.jgrapes.core.Channel; import org.jgrapes.core.EventPipeline; @@ -142,7 +142,8 @@ public class PoolMonitor extends * @throws ApiException */ @Handler - public void onVmDefChanged(VmDefChanged event) throws ApiException { + public void onVmResourceChanged(VmResourceChanged event) + throws ApiException { final var vmDef = event.vmDefinition(); final String vmName = vmDef.name(); switch (event.type()) { diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PvcReconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PvcReconciler.java index 107dca7..47aa8be 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PvcReconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PvcReconciler.java @@ -67,30 +67,35 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; /** * Reconcile the PVCs. * - * @param vmDef the vm def + * @param vmDef the VM definition * @param model the model * @param channel the channel + * @param specChanged the spec changed * @throws IOException Signals that an I/O exception has occurred. * @throws TemplateException the template exception * @throws ApiException the api exception */ - @SuppressWarnings("PMD.AvoidDuplicateLiterals") + @SuppressWarnings({ "PMD.AvoidDuplicateLiterals", "unchecked" }) public void reconcile(VmDefinition vmDef, Map model, - VmChannel channel) + VmChannel channel, boolean specChanged) throws IOException, TemplateException, ApiException { - // Existing disks - ListOptions listOpts = new ListOptions(); - listOpts.setLabelSelector( - "app.kubernetes.io/managed-by=" + VM_OP_NAME + "," - + "app.kubernetes.io/name=" + APP_NAME + "," - + "app.kubernetes.io/instance=" + vmDef.name()); - var knownDisks = K8sV1PvcStub.list(channel.client(), - vmDef.namespace(), listOpts); - var knownPvcs = knownDisks.stream().map(K8sV1PvcStub::name) - .collect(Collectors.toSet()); + Set knownPvcs; + if (!specChanged && channel.associated(this, Set.class).isPresent()) { + knownPvcs = (Set) channel.associated(this, Set.class).get(); + } else { + ListOptions listOpts = new ListOptions(); + listOpts.setLabelSelector( + "app.kubernetes.io/managed-by=" + VM_OP_NAME + "," + + "app.kubernetes.io/name=" + APP_NAME + "," + + "app.kubernetes.io/instance=" + vmDef.name()); + knownPvcs = K8sV1PvcStub.list(channel.client(), + vmDef.namespace(), listOpts).stream().map(K8sV1PvcStub::name) + .collect(Collectors.toSet()); + channel.setAssociated(this, knownPvcs); + } // Reconcile runner data pvc - reconcileRunnerDataPvc(vmDef, model, channel, knownPvcs); + reconcileRunnerDataPvc(vmDef, model, channel, knownPvcs, specChanged); // Reconcile pvcs for defined disks var diskDefs = vmDef.>> fromVm("disks") @@ -114,15 +119,13 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; } // Update PVC - model.put("disk", diskDef); - reconcileRunnerDiskPvc(vmDef, model, channel); + reconcileRunnerDiskPvc(vmDef, model, channel, specChanged, diskDef); } - model.remove("disk"); } private void reconcileRunnerDataPvc(VmDefinition vmDef, Map model, VmChannel channel, - Set knownPvcs) + Set knownPvcs, boolean specChanged) throws TemplateNotFoundException, MalformedTemplateNameException, ParseException, IOException, TemplateException, ApiException { @@ -135,7 +138,12 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; } // Generate PVC - model.put("runnerDataPvcName", vmDef.name() + "-runner-data"); + var runnerDataPvcName = vmDef.name() + "-runner-data"; + model.put("runnerDataPvcName", runnerDataPvcName); + if (!specChanged) { + // Augmenting the model is all we have to do + return; + } var fmTemplate = fmConfig.getTemplate("runnerDataPvc.ftl.yaml"); StringWriter out = new StringWriter(); fmTemplate.process(model, out); @@ -159,17 +167,24 @@ import org.yaml.snakeyaml.constructor.SafeConstructor; } private void reconcileRunnerDiskPvc(VmDefinition vmDef, - Map model, VmChannel channel) + Map model, VmChannel channel, boolean specChanged, + Map diskDef) throws TemplateNotFoundException, MalformedTemplateNameException, ParseException, IOException, TemplateException, ApiException { // Generate PVC - @SuppressWarnings("unchecked") - var diskDef = (Map) model.get("disk"); var pvcName = vmDef.name() + "-" + diskDef.get("generatedDiskName"); diskDef.put("generatedPvcName", pvcName); + if (!specChanged) { + // Augmenting the model is all we have to do + return; + } + + // Generate PVC + model.put("disk", diskDef); var fmTemplate = fmConfig.getTemplate("runnerDiskPvc.ftl.yaml"); StringWriter out = new StringWriter(); fmTemplate.process(model, out); + model.remove("disk"); // Avoid Yaml.load due to // https://github.com/kubernetes-client/java/issues/2741 var pvcDef = Dynamics.newFromYaml( diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java index 170263e..700b081 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/Reconciler.java @@ -44,15 +44,13 @@ import java.util.Optional; import java.util.logging.Level; import org.jdrupes.vmoperator.common.Convertions; import org.jdrupes.vmoperator.common.K8sObserver; -import org.jdrupes.vmoperator.common.K8sObserver.ResponseType; import org.jdrupes.vmoperator.common.VmDefinition; import org.jdrupes.vmoperator.common.VmDefinition.Assignment; import org.jdrupes.vmoperator.common.VmPool; import org.jdrupes.vmoperator.manager.events.GetPools; -import org.jdrupes.vmoperator.manager.events.PodChanged; import org.jdrupes.vmoperator.manager.events.ResetVm; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jdrupes.vmoperator.util.ExtendedObjectWrapper; import org.jgrapes.core.Channel; import org.jgrapes.core.Component; @@ -151,7 +149,6 @@ public class Reconciler extends Component { private final Configuration fmConfig; private final ConfigMapReconciler cmReconciler; private final DisplaySecretReconciler dsReconciler; - private final StatefulSetReconciler stsReconciler; private final PvcReconciler pvcReconciler; private final PodReconciler podReconciler; private final LoadBalancerReconciler lbReconciler; @@ -179,7 +176,6 @@ public class Reconciler extends Component { cmReconciler = new ConfigMapReconciler(fmConfig); dsReconciler = attach(new DisplaySecretReconciler(componentChannel)); - stsReconciler = new StatefulSetReconciler(fmConfig); pvcReconciler = new PvcReconciler(fmConfig); podReconciler = new PodReconciler(fmConfig); lbReconciler = new LoadBalancerReconciler(fmConfig); @@ -208,64 +204,27 @@ public class Reconciler extends Component { */ @Handler @SuppressWarnings("PMD.ConfusingTernary") - public void onVmDefChanged(VmDefChanged event, VmChannel channel) + public void onVmResourceChanged(VmResourceChanged event, VmChannel channel) throws ApiException, TemplateException, IOException { // Ownership relationships takes care of deletions if (event.type() == K8sObserver.ResponseType.DELETED) { return; } - // Reconcile - reconcile(event, channel); - } - - private void reconcile(VmDefChanged event, VmChannel channel) - throws TemplateModelException, ApiException, IOException, - TemplateException { // Create model for processing templates var vmDef = event.vmDefinition(); Map model = prepareModel(vmDef); cmReconciler.reconcile(model, channel, event.specChanged()); - // The remaining reconcilers depend only on changes of the spec part. - if (!event.specChanged()) { + // The remaining reconcilers depend only on changes of the spec part + // or the pod state. + if (!event.specChanged() && !event.podChanged()) { return; } - dsReconciler.reconcile(vmDef, model, channel); - // Manage (eventual) removal of stateful set. - stsReconciler.reconcile(vmDef, model, channel); - pvcReconciler.reconcile(vmDef, model, channel); - podReconciler.reconcile(vmDef, model, channel); - lbReconciler.reconcile(vmDef, model, channel); - } - - /** - * On pod changed. - * - * @param event the event - * @param channel the channel - * @throws ApiException the api exception - * @throws IOException Signals that an I/O exception has occurred. - * @throws TemplateException the template exception - */ - @Handler - public void onPodChanged(PodChanged event, VmChannel channel) - throws ApiException, IOException, TemplateException { - if (event.type() != ResponseType.DELETED) { - // Nothing to reconcile - return; - } - - // If the pod was deleted, it may be necessary to recreate it - var vmDef = channel.vmDefinition(); - Map model = prepareModel(vmDef); - - // Call all steps because they may augment the model - cmReconciler.reconcile(model, channel, false); - dsReconciler.reconcile(vmDef, model, channel); - stsReconciler.reconcile(vmDef, model, channel); - pvcReconciler.reconcile(vmDef, model, channel); - podReconciler.reconcile(vmDef, model, channel); + dsReconciler.reconcile(vmDef, model, channel, event.specChanged()); + pvcReconciler.reconcile(vmDef, model, channel, event.specChanged()); + podReconciler.reconcile(vmDef, model, channel, event.specChanged()); + lbReconciler.reconcile(vmDef, model, channel, event.specChanged()); } /** @@ -282,7 +241,8 @@ public class Reconciler extends Component { public void onResetVm(ResetVm event, VmChannel channel) throws ApiException, IOException, TemplateException { var vmDef = channel.vmDefinition(); - vmDef.extra().ifPresent(e -> e.resetCount(e.resetCount() + 1)); + var extra = vmDef.extra(); + extra.resetCount(extra.resetCount() + 1); Map model = prepareModel(channel.vmDefinition()); cmReconciler.reconcile(model, channel, true); diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/StatefulSetReconciler.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/StatefulSetReconciler.java deleted file mode 100644 index 854f630..0000000 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/StatefulSetReconciler.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * VM-Operator - * Copyright (C) 2023 Michael N. Lipp - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -package org.jdrupes.vmoperator.manager; - -import freemarker.template.Configuration; -import freemarker.template.TemplateException; -import io.kubernetes.client.custom.V1Patch; -import io.kubernetes.client.openapi.ApiException; -import io.kubernetes.client.util.generic.options.PatchOptions; -import java.io.IOException; -import java.util.Map; -import java.util.logging.Logger; -import org.jdrupes.vmoperator.common.K8sV1StatefulSetStub; -import org.jdrupes.vmoperator.common.VmDefinition; -import org.jdrupes.vmoperator.common.VmDefinition.RequestedVmState; -import org.jdrupes.vmoperator.manager.events.VmChannel; - -/** - * Before version 3.4, the pod running the VM was created by a stateful set. - * Starting with version 3.4, this reconciler simply deletes the stateful - * set, provided that the VM is not running. - */ -@SuppressWarnings("PMD.DataflowAnomalyAnalysis") -/* default */ class StatefulSetReconciler { - - protected final Logger logger = Logger.getLogger(getClass().getName()); - - /** - * Instantiates a new stateful set reconciler. - * - * @param fmConfig the fm config - */ - @SuppressWarnings("PMD.UnusedFormalParameter") - public StatefulSetReconciler(Configuration fmConfig) { - // Nothing to do - } - - /** - * Reconcile stateful set. - * - * @param vmDef the VM definition - * @param model the model - * @param channel the channel - * @throws IOException Signals that an I/O exception has occurred. - * @throws TemplateException the template exception - * @throws ApiException the api exception - */ - @SuppressWarnings("PMD.AvoidLiteralsInIfCondition") - public void reconcile(VmDefinition vmDef, Map model, - VmChannel channel) - throws IOException, TemplateException, ApiException { - model.put("usingSts", false); - - // If exists, delete when not running or supposed to be not running. - var stsStub = K8sV1StatefulSetStub.get(channel.client(), - vmDef.namespace(), vmDef.name()); - if (stsStub.model().isEmpty()) { - return; - } - - // Stateful set still exists, check if replicas is 0 so we can - // delete it. - var stsModel = stsStub.model().get(); - if (stsModel.getSpec().getReplicas() == 0) { - stsStub.delete(); - return; - } - - // Cannot yet delete the stateful set. - model.put("usingSts", true); - - // Check if VM is supposed to be stopped. If so, - // set replicas to 0. This is the first step of the transition, - // the stateful set will be deleted when the VM is restarted. - if (vmDef.vmState() == RequestedVmState.RUNNING) { - return; - } - - // Do apply changes (set replicas to 0) - PatchOptions opts = new PatchOptions(); - opts.setForce(true); - opts.setFieldManager("kubernetes-java-kubectl-apply"); - if (stsStub.patch(V1Patch.PATCH_FORMAT_JSON_PATCH, - new V1Patch("[{\"op\": \"replace\", \"path\": \"/spec/replicas" - + "\", \"value\": 0}]"), - channel.client().defaultPatchOptions()).isEmpty()) { - logger.warning( - () -> "Could not patch stateful set for " + stsStub.name()); - } - } -} diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java index 9e6e5c9..09bade8 100644 --- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java +++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java @@ -30,7 +30,6 @@ import java.net.HttpURLConnection; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -54,7 +53,7 @@ import org.jdrupes.vmoperator.manager.events.ModifyVm; import org.jdrupes.vmoperator.manager.events.PodChanged; import org.jdrupes.vmoperator.manager.events.UpdateAssignment; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jdrupes.vmoperator.util.GsonPtr; import org.jgrapes.core.Channel; import org.jgrapes.core.Event; @@ -157,11 +156,11 @@ public class VmMonitor extends // Create and fire changed event. Remove channel from channel // manager on completion. - VmDefChanged chgEvt - = new VmDefChanged(ResponseType.valueOf(response.type), + VmResourceChanged chgEvt + = new VmResourceChanged(ResponseType.valueOf(response.type), vmDef, channel.setGeneration(response.object.getMetadata() .getGeneration()), - vmDef); + false); if (ResponseType.valueOf(response.type) == ResponseType.DELETED) { chgEvt = Event.onCompletion(chgEvt, e -> channelManager.remove(e.vmDefinition().name())); @@ -181,8 +180,7 @@ public class VmMonitor extends @SuppressWarnings("PMD.AvoidDuplicateLiterals") private void addExtraData(VmDefinition vmDef, VmDefinition prevState) { var extra = new VmExtraData(vmDef); - var prevExtra - = Optional.ofNullable(prevState).flatMap(VmDefinition::extra); + var prevExtra = Optional.ofNullable(prevState).map(VmDefinition::extra); // Maintain (or initialize) the resetCount extra.resetCount(prevExtra.map(VmExtraData::resetCount).orElse(0L)); @@ -200,35 +198,35 @@ public class VmMonitor extends */ @Handler public void onPodChanged(PodChanged event, VmChannel channel) { - if (channel.vmDefinition().extra().isEmpty()) { - return; - } - var extra = channel.vmDefinition().extra().get(); - var pod = event.pod(); + var vmDef = channel.vmDefinition(); + updateNodeInfo(event, vmDef); + channel + .fire(new VmResourceChanged(ResponseType.MODIFIED, vmDef, false, true)); + } + + private void updateNodeInfo(PodChanged event, VmDefinition vmDef) { + var extra = vmDef.extra(); if (event.type() == ResponseType.DELETED) { // The status of a deleted pod is the status before deletion, - // i.e. the node info is still there. + // i.e. the node info is still cached and must be removed. extra.nodeInfo("", Collections.emptyList()); - } else { - var nodeName = Optional - .ofNullable(pod.getSpec().getNodeName()).orElse(""); - logger.finer(() -> "Adding node name " + nodeName - + " to VM info for " + channel.vmDefinition().name()); - @SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops") - var addrs = new ArrayList(); - Optional.ofNullable(pod.getStatus().getPodIPs()) - .orElse(Collections.emptyList()).stream() - .map(ip -> ip.getIp()).forEach(addrs::add); - logger.finer(() -> "Adding node addresses " + addrs - + " to VM info for " + channel.vmDefinition().name()); - if (Objects.equals(nodeName, extra.nodeName()) - && Objects.equals(addrs, extra.nodeAddresses())) { - return; - } - extra.nodeInfo(nodeName, addrs); + return; } - channel.fire(new VmDefChanged(ResponseType.MODIFIED, false, - channel.vmDefinition())); + + // Get current node info from pod + var pod = event.pod(); + var nodeName = Optional + .ofNullable(pod.getSpec().getNodeName()).orElse(""); + logger.finer(() -> "Adding node name " + nodeName + + " to VM info for " + vmDef.name()); + @SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops") + var addrs = new ArrayList(); + Optional.ofNullable(pod.getStatus().getPodIPs()) + .orElse(Collections.emptyList()).stream() + .map(ip -> ip.getIp()).forEach(addrs::add); + logger.finer(() -> "Adding node addresses " + addrs + + " to VM info for " + vmDef.name()); + extra.nodeInfo(nodeName, addrs); } /** diff --git a/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java b/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java index d3de96e..b48aa7e 100644 --- a/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java +++ b/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java @@ -57,8 +57,8 @@ import org.jdrupes.vmoperator.manager.events.GetVms.VmData; import org.jdrupes.vmoperator.manager.events.ModifyVm; import org.jdrupes.vmoperator.manager.events.ResetVm; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; import org.jdrupes.vmoperator.manager.events.VmPoolChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jgrapes.core.Channel; import org.jgrapes.core.Components; import org.jgrapes.core.Event; @@ -658,7 +658,7 @@ public class VmAccess extends FreeMarkerConlet { @SuppressWarnings({ "PMD.ConfusingTernary", "PMD.CognitiveComplexity", "PMD.AvoidInstantiatingObjectsInLoops", "PMD.AvoidDuplicateLiterals", "PMD.ConfusingArgumentToVarargsMethod" }) - public void onVmDefChanged(VmDefChanged event, VmChannel channel) + public void onVmResourceChanged(VmResourceChanged event, VmChannel channel) throws IOException, InterruptedException { var vmDef = event.vmDefinition(); @@ -847,8 +847,8 @@ public class VmAccess extends FreeMarkerConlet { if (!event.secretAvailable()) { return; } - vmDef.extra().map(xtra -> xtra.connectionFile(event.secret(), - preferredIpVersion, deleteConnectionFile)) + vmDef.extra().connectionFile(event.secret(), + preferredIpVersion, deleteConnectionFile) .ifPresent(cf -> channel.respond(new NotifyConletView(type(), model.getConletId(), "openConsole", cf))); } diff --git a/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java b/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java index f1c5d70..97d6867 100644 --- a/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java +++ b/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java @@ -42,13 +42,12 @@ import org.jdrupes.vmoperator.common.Constants.Status; import org.jdrupes.vmoperator.common.K8sObserver; import org.jdrupes.vmoperator.common.VmDefinition; import org.jdrupes.vmoperator.common.VmDefinition.Permission; -import org.jdrupes.vmoperator.common.VmExtraData; import org.jdrupes.vmoperator.manager.events.ChannelTracker; import org.jdrupes.vmoperator.manager.events.GetDisplaySecret; import org.jdrupes.vmoperator.manager.events.ModifyVm; import org.jdrupes.vmoperator.manager.events.ResetVm; import org.jdrupes.vmoperator.manager.events.VmChannel; -import org.jdrupes.vmoperator.manager.events.VmDefChanged; +import org.jdrupes.vmoperator.manager.events.VmResourceChanged; import org.jdrupes.vmoperator.util.DataPath; import org.jgrapes.core.Channel; import org.jgrapes.core.Event; @@ -258,7 +257,7 @@ public class VmMgmt extends FreeMarkerConlet { "name", vmDef.name()), "spec", spec, "status", status, - "nodeName", vmDef.extra().map(VmExtraData::nodeName).orElse(""), + "nodeName", vmDef.extra().nodeName(), "consoleAccessible", vmDef.consoleAccessible(user, perms), "permissions", perms); } @@ -274,7 +273,7 @@ public class VmMgmt extends FreeMarkerConlet { @SuppressWarnings({ "PMD.ConfusingTernary", "PMD.CognitiveComplexity", "PMD.AvoidInstantiatingObjectsInLoops", "PMD.AvoidDuplicateLiterals", "PMD.ConfusingArgumentToVarargsMethod" }) - public void onVmDefChanged(VmDefChanged event, VmChannel channel) + public void onVmResourceChanged(VmResourceChanged event, VmChannel channel) throws IOException { var vmName = event.vmDefinition().name(); if (event.type() == K8sObserver.ResponseType.DELETED) { @@ -494,8 +493,8 @@ public class VmMgmt extends FreeMarkerConlet { if (!event.secretAvailable()) { return; } - vmDef.extra().map(xtra -> xtra.connectionFile(event.secret(), - preferredIpVersion, deleteConnectionFile)).ifPresent( + vmDef.extra().connectionFile(event.secret(), + preferredIpVersion, deleteConnectionFile).ifPresent( cf -> channel.respond(new NotifyConletView(type(), model.getConletId(), "openConsole", cf))); } diff --git a/webpages/upgrading.md b/webpages/upgrading.md index a590bcc..644dce6 100644 --- a/webpages/upgrading.md +++ b/webpages/upgrading.md @@ -34,6 +34,10 @@ layout: vm-operator update the template manually. If you're using your own template, you have to add a virtual serial port (see the git history of the standard template for the required addition). + + * Stateful sets from pre 3.4.0 versions are no longer removed automatically + (see notes below). However, PVCs with the old naming scheme are still + reused. ## To version 3.4.0