Update application shutdown logic #1760

Yavor16 · 2026-01-05T09:21:11Z

No description provided.

LMCROSSITXSADEPLOY-3344 # Conflicts: # multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java

sonarqubecloud · 2026-01-05T09:30:54Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

theghost5800 · 2026-01-06T12:23:02Z

...loudfoundry/multiapps/controller/core/application/shutdown/ApplicationShutdownScheduler.java

+        return ImmutableApplicationShutdown.builder()
+                                           .id(UUID.randomUUID()
+                                                   .toString())
+                                           .staredAt(Date.from(Instant.now()))


staredAt or startedAt? Is there a typo?

theghost5800 · 2026-01-06T12:37:25Z

...iapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java

-    public static final String APP_SHUTDOWNED = "Application with id:\"{0}\", instance id:\"{1}\", instance index:\"{2}\", is shutdowned. Timeout to wait before shutdown of Flowable job executor:\"{3}\" seconds.";
-    public static final String APP_SHUTDOWN_STATUS_MONITOR = "Monitor shutdown status of application with id:\"{0}\", instance id:\"{1}\", instance index:\"{2}\". Status:\"{3}\".";
+    public static final String APP_SHUTDOWN_STATUS_MONITOR = "Monitor shutdown status of application with id:\"{0}\"";
+    public static final String APP_INSTANCE_WITH_ID_AND_INDEX_SCHEDULED_FOR_DELETION = "Application with ID \"{0}\" and index \"{1}\" has been scheduled for deletion";


Shouldn't be scheduled for stop/shutdown?

theghost5800 · 2026-01-06T12:44:28Z

...loudfoundry/multiapps/controller/core/application/shutdown/ApplicationShutdownScheduler.java

@@ -0,0 +1,56 @@
+package org.cloudfoundry.multiapps.controller.core.application.shutdown;


Isn't it better to keep all logic related to shutdown in the separate java package/maven submodule?

theghost5800 · 2026-01-06T12:45:32Z

...src/main/java/org/cloudfoundry/multiapps/controller/persistence/dto/ApplicationShutdown.java

    @Value.Default
-    default Status getStatus() {
-        return Status.RUNNING;
+    default String getStatus() {


Why don't preserve return of enum object instead of string?

theghost5800 · 2026-01-06T12:53:09Z

.../main/java/org/cloudfoundry/multiapps/controller/persistence/dto/ApplicationShutdownDto.java

+import jakarta.persistence.TemporalType;
+
+@Entity
+@Table(name = "application_shutdown")


Extract as constants into PersistenceMetadata

theghost5800 · 2026-01-07T07:41:49Z

...t/src/main/java/org/cloudfoundry/multiapps/controller/shutdown/client/util/ShutdownUtil.java

+    }
+
+    public static boolean isTimeoutExceeded(ApplicationShutdown applicationShutdown) {
+        Instant thirtyMinutesAfterStartedDate = Instant.from(applicationShutdown.getStaredAt()


How this is 30 minutes after start when timeout constant says 10 minutes?

theghost5800 · 2026-01-07T08:46:47Z

...t/src/main/java/org/cloudfoundry/multiapps/controller/shutdown/client/util/ShutdownUtil.java

+        return timeNow.isAfter(thirtyMinutesAfterStartedDate);
+    }
+
+    public static void print(List<ApplicationShutdown> shutdown) {


Can you name the method with more meaningful name - for example printShutdownStatus or logShutdownStatus. Also name shutdown variable to include instances in the name.

theghost5800 · 2026-01-07T09:01:58Z

.../java/org/cloudfoundry/multiapps/controller/shutdown/client/ApplicationShutdownExecutor.java

-            instanceShutdownExecutor.execute(applicationGuid, i);
+    public void execute(String applicationId, int applicationInstanceCount) {
+        ApplicationShutdownScheduler applicationShutdownScheduler = getApplicationShutdownScheduler();
+        List<ApplicationShutdown> scheduledApplicationShutdowns = applicationShutdownScheduler.scheduleApplicationForShutdown(applicationId,


What will happen if count of instances is mistaken or some of existing instances are in crashed/not running state? Will it lead to infinite loop?

theghost5800 · 2026-01-07T09:14:38Z

...in/java/org/cloudfoundry/multiapps/controller/web/resources/ApplicationShutdownResource.java

-import jakarta.inject.Inject;
-
-@RestController
-@RequestMapping(value = Constants.Resources.APPLICATION_SHUTDOWN)


Do you delete all related constants?

theghost5800 · 2026-01-07T09:15:52Z

pom.xml

                <groupId>jakarta.servlet</groupId>
                <artifactId>jakarta.servlet-api</artifactId>
                <version>${servlet-api.version}</version>
-                <scope>provided</scope>


Why this dependency is now included as part of the war file? Isn't it provided by the java buildpack?

stiv03 · 2026-01-07T09:57:29Z

...-web/src/main/java/org/cloudfoundry/multiapps/controller/web/bootstrap/BootstrapServlet.java

+        int applicationIndex = configuration.getApplicationInstanceIndex();
+        ApplicationShutdown applicationShutdown = applicationShutdownService.getApplicationsByApplicationIndex(applicationIndex);
+        if (applicationShutdown != null) {
+            LOGGER.info(org.cloudfoundry.multiapps.controller.core.Messages.CLEARING_OLD_ENTRY + applicationIndex);


Use here MessageFormat.format instead of concatenation

stiv03 · 2026-01-07T10:09:44Z

...loudfoundry/multiapps/controller/core/application/shutdown/ApplicationShutdownScheduler.java

+                                                                                 List<String> instancesIds) {
+        List<ApplicationShutdown> instances = new ArrayList<>();
+        for (String instanceId : instancesIds) {
+            instances.add(applicationShutdownService.getApplicationShutdownInstanceByInstanceId(instanceId, applicationId));


Can this return null?

stiv03 · 2026-01-07T10:19:22Z

...a/org/cloudfoundry/multiapps/controller/persistence/services/ApplicationShutdownService.java

+    public ApplicationShutdown updateApplicationShutdownStatus(ApplicationShutdown oldApplicationShutdown, String status) {
+        ApplicationShutdown newApplicationShutdown = ImmutableApplicationShutdown.copyOf(oldApplicationShutdown)
+                                                                                 .withStatus(status);


Maybe enum for status instead of String

stiv03 · 2026-01-07T10:26:12Z

.../main/java/org/cloudfoundry/multiapps/controller/persistence/dto/ApplicationShutdownDto.java

+
+    @Column(name = "application_instance_index", nullable = false, unique = true)
+    private int applicationInstanceIndex;
+
+    @Column(name = "application_id", nullable = false)
+    private String applicationId;
+
+    @Column(name = "shutdown_status", nullable = false)
+    private String shutdownStatus;
+
+    @Column(name = "started_at", nullable = false)
+    @Temporal(TemporalType.TIMESTAMP)
+    private Date startedAt;


extract as constants

stiv03 · 2026-01-07T10:33:20Z

...g/cloudfoundry/multiapps/controller/persistence/query/impl/ApplicationShutdownQueryImpl.java

+    }
+
+    @Override
+    public ApplicationShutdown singleResult() throws NoResultException, NonUniqueResultException {


Doesn't this method return null, why throw NoResultException

stiv03 · 2026-01-08T09:16:00Z

...src/main/java/org/cloudfoundry/multiapps/controller/process/jobs/ApplicationShutdownJob.java

+        if (applicationShutdown == null || applicationShutdown.getStatus()
+                                                              .equals(Status.FINISHED.name())) {


Here use directly the enum instead of string

stiv03 · 2026-01-08T09:24:23Z

...src/main/java/org/cloudfoundry/multiapps/controller/process/jobs/ApplicationShutdownJob.java

+    private void shutdownApplication(ApplicationShutdown applicationShutdown) {
+        CompletableFuture.runAsync(() -> {
+                             logProgressOfShuttingDown(applicationShutdown, Messages.SHUTTING_DOWN_APPLICATION_WITH_ID_AND_INDEX);
+                             flowableFacade.shutdownJobExecutor();
+                         })
+                         .thenRun(() -> logProgressOfShuttingDown(applicationShutdown, Messages.SHUT_DOWN_APPLICATION_WITH_ID_AND_INDEX));
+    }


Is it possible here, exception to be thrown inside the async task, this will impact the DB state in a wrong way. Maybe add FAILED status for such cases.

stiv03 · 2026-01-08T09:50:33Z

Also add some unit tests, cause now there is 0.0% Coverage on New Code

Yavor16 force-pushed the application-shutdown-logic-update branch from 0642e66 to 31b61f2 Compare January 5, 2026 09:24

Update application shutdown logic

8da103c

LMCROSSITXSADEPLOY-3344 # Conflicts: # multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java

Yavor16 force-pushed the application-shutdown-logic-update branch from 31b61f2 to 8da103c Compare January 5, 2026 09:25

theghost5800 reviewed Jan 7, 2026

View reviewed changes

stiv03 suggested changes Jan 8, 2026

View reviewed changes

		@@ -0,0 +1,56 @@
		package org.cloudfoundry.multiapps.controller.core.application.shutdown;

		if (applicationShutdown == null \|\| applicationShutdown.getStatus()
		.equals(Status.FINISHED.name())) {

Update application shutdown logic #1760

Are you sure you want to change the base?

Update application shutdown logic #1760

Conversation

Yavor16 commented Jan 5, 2026

Uh oh!

sonarqubecloud bot commented Jan 5, 2026

Quality Gate failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stiv03 commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants