Support SKU spec upgrade #459

Andyz26 · 2023-06-12T22:10:56Z

Context

Currently upgrade operation on RC is for the image version.
Support a new flag in upgrade RC to be able to upgrade Sku spec in upgrade operation.

Checklist

./gradlew build compiles code correctly
Added new tests where applicable
./gradlew test passes all tests
Extended README or added javadocs where applicable

github-actions · 2023-06-12T22:22:52Z

Test Results

127 files ±0 127 suites ±0 6m 24s ⏱️ ±0s
538 tests +1 528 ✔️ ±0 8 💤 ±0 2 ❌ +1
540 runs +2 530 ✔️ +1 8 💤 ±0 2 ❌ +1

For more details on these failures, see this check.

Results for commit e179555. ± Comparison against base commit b119679.

♻️ This comment has been updated with latest results.

github-actions · 2023-06-12T22:26:14Z

Uploaded Artifacts

To use these artifacts in your Gradle project, paste the following lines in your build.gradle.

resolutionStrategy {
    force "io.mantisrx:mantis-client:0.1.0-20230613.173850-288"
    force "io.mantisrx:mantis-common:0.1.0-20230613.173850-287"
    force "io.mantisrx:mantis-common-serde:0.1.0-20230613.173850-287"
    force "io.mantisrx:mantis-discovery-proto:0.1.0-20230613.173850-287"
    force "io.mantisrx:mantis-network:0.1.0-20230613.173850-287"
    force "io.mantisrx:mantis-remote-observable:0.1.0-20230613.173850-288"
    force "io.mantisrx:mantis-runtime:0.1.0-20230613.173850-288"
    force "io.mantisrx:mantis-runtime-loader:0.1.0-20230613.173850-288"
    force "io.mantisrx:mantis-shaded:0.1.0-20230613.173850-286"
    force "io.mantisrx:mantis-connector-iceberg:0.1.0-20230613.173850-286"
    force "io.mantisrx:mantis-connector-job:0.1.0-20230613.173850-288"
    force "io.mantisrx:mantis-connector-kafka:0.1.0-20230613.173850-288"
    force "io.mantisrx:mantis-connector-publish:0.1.0-20230613.173850-287"
    force "io.mantisrx:mantis-control-plane-client:0.1.0-20230613.173850-287"
    force "io.mantisrx:mantis-control-plane-core:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-control-plane-server:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-core:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-groupby-sample:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-jobconnector-sample:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-mantis-publish-sample:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-sine-function:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-synthetic-sourcejob:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-twitter-sample:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-examples-wordcount:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-publish-core:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-publish-netty:0.1.0-20230613.173850-280"
    force "io.mantisrx:mantis-publish-netty-guice:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-server-agent:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-server-worker:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-server-worker-client:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-source-job-kafka:0.1.0-20230613.173850-281"
    force "io.mantisrx:mantis-source-job-publish:0.1.0-20230613.173850-281"
}

sundargates · 2023-06-13T05:54:56Z

.../src/main/java/io/mantisrx/master/resourcecluster/proto/UpgradeClusterContainersRequest.java

+    @Nullable
+    MantisResourceClusterSpec resourceClusterSpec;


nit: why do we need this here? Do we not have a separate endpoint to update the cluster spec?

I was thinking along the lines of:

resourceCluster.sku(SKU).clusterSpec(NEW_SPEC).image(IMAGE).reconcile()

this is served as a translation between actors to Temporal workflow invoker. It's probably cleaner to have another intermediate contract class here instead.

sundargates · 2023-06-13T05:57:26Z

...erver/src/main/java/io/mantisrx/master/resourcecluster/ResourceClustersHostManagerActor.java

+            upgradeFut = this.resourceClusterStorageProvider.getResourceClusterSpecWritable(req.getClusterId())
+                .thenCompose(specW -> {
+                    if (specW == null) {
+                        return CompletableFuture.completedFuture(UpgradeClusterContainersResponse.builder()
+                            .responseCode(ResponseCode.CLIENT_ERROR_NOT_FOUND)
+                            .build());
+                    }
+
+                    UpgradeClusterContainersRequest enrichedReq =
+                        req.toBuilder()
+                            .resourceClusterSpec(specW.getClusterSpec())
+                            .build();
+                    return this.resourceClusterProvider.upgradeContainerResource(enrichedReq);
+                })


I think ideally we should not have this here as it increases the complexity of this endpoint.

Temporal workflow doesn't understand how to get the spec from internal kvDal translation. So the choices are between:

Have workflow calling back to master endpoint to GET resource spec then apply it in the workflow.

Have the actor include the spec to send to workflow invoker directly (current). I think this is cleaner and less interactions needed.

sundargates · 2023-06-13T20:31:17Z

...erver/src/main/java/io/mantisrx/master/resourcecluster/ResourceClustersHostManagerActor.java

-
-        pipe(this.resourceClusterProvider.upgradeContainerResource(req), getContext().dispatcher()).to(getSender());
+        // For scaling-down the decision requires getting idle hosts first.
+        // if enableSkuSpecUpgrade is true, first fetch the latest spec to override the sku spec during upgrade


Instead of fetching this, could we send this as an argument to the workflow?

it is sending the spec over the wire.

sundargates · 2023-06-13T20:32:25Z

.../mantisrx/master/resourcecluster/resourceprovider/ResourceClusterProviderUpgradeRequest.java

+
+    String region;
+
+    String optionalImageId;


This need not be optional, correct?

I think it current default to "latest"

sundargates · 2023-06-13T20:33:50Z

.../mantisrx/master/resourcecluster/resourceprovider/ResourceClusterProviderUpgradeRequest.java

+
+    int optionalBatchMaxSize;
+
+    boolean forceUpgradeOnSameImage;


s/forceUpgradeOnSameImage/force

sundargates · 2023-06-13T20:34:13Z

.../mantisrx/master/resourcecluster/resourceprovider/ResourceClusterProviderUpgradeRequest.java

+
+    MantisResourceClusterEnvType optionalEnvType;
+
+    int optionalBatchMaxSize;


what is this batchMaxSize for?

Per upgrade batch settings where each batch in upgrade should not have more than x agents involved.

Support sku spec upgrade

80fa9fd

Andyz26 requested review from nickmahilani, jeffchao, piygoyal, calvin681, sundargates, hmitnflx, markcho, liuml07 and fdc-ntflx as code owners June 12, 2023 22:10

Andyz26 temporarily deployed to Integrate Pull Request June 12, 2023 22:11 — with GitHub Actions Inactive

Andyz26 changed the title ~~[WIP, Do not review] Support SKU spec upgrade~~ Support SKU spec upgrade Jun 13, 2023

sundargates reviewed Jun 13, 2023

View reviewed changes

separate UpgradeReq to workflow

75180d3

Andyz26 temporarily deployed to Integrate Pull Request June 13, 2023 17:38 — with GitHub Actions Inactive

sundargates reviewed Jun 13, 2023

View reviewed changes

sundargates approved these changes Jun 13, 2023

View reviewed changes

annotate nullable fields

e179555

Andyz26 had a problem deploying to Integrate Pull Request June 13, 2023 21:37 — with GitHub Actions Failure

Andyz26 merged commit 16ae78d into master Jun 13, 2023
1 of 3 checks passed

Andyz26 deleted the andyz/upgradeWithSkuSpec branch July 10, 2023 05:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support SKU spec upgrade #459

Support SKU spec upgrade #459

Andyz26 commented Jun 12, 2023 •

edited

Loading

github-actions bot commented Jun 12, 2023 •

edited

Loading

github-actions bot commented Jun 12, 2023 •

edited

Loading

sundargates Jun 13, 2023

sundargates Jun 13, 2023

Andyz26 Jun 13, 2023 •

edited

Loading

sundargates Jun 13, 2023

Andyz26 Jun 13, 2023

sundargates Jun 13, 2023

Andyz26 Jun 13, 2023

sundargates Jun 13, 2023

Andyz26 Jun 13, 2023

sundargates Jun 13, 2023

sundargates Jun 13, 2023

Andyz26 Jun 13, 2023


		MantisResourceClusterEnvType optionalEnvType;

		int optionalBatchMaxSize;

Support SKU spec upgrade #459

Support SKU spec upgrade #459

Conversation

Andyz26 commented Jun 12, 2023 • edited Loading

Context

Checklist

github-actions bot commented Jun 12, 2023 • edited Loading

Test Results

github-actions bot commented Jun 12, 2023 • edited Loading

Uploaded Artifacts

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Andyz26 Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Andyz26 commented Jun 12, 2023 •

edited

Loading

github-actions bot commented Jun 12, 2023 •

edited

Loading

github-actions bot commented Jun 12, 2023 •

edited

Loading

Andyz26 Jun 13, 2023 •

edited

Loading