feat: add --skew flag and keep a single workload without it

README.md

@@ -166,18 +166,24 @@ Deploy a second version alongside the first:
 desk deploy --profile skew-protection --dir ./my-watt-project --version v2
 ```
 
-When `--version` / `-v` is provided, the Deployment and Service are named
-`{app}-{version}` (e.g., `my-watt-project-v1`) and labelled with
-`app.kubernetes.io/name: my-watt-project` and `plt.dev/version: v1`. Traffic
-routes through Gateway API HTTPRoutes managed by ICC.
-
-Without `--version`, `desk` leaves the version unset and lets ICC mint it: ICC
-derives a `plt_` id from the image (same shape as a Vercel deployment id, with a
-`plt_` prefix) and delivers it back to the pod. The workload is then named
-`{app}-{image-tag}` (unique per build) instead of `{app}`, so two deploys from
-different images coexist as separate Deployments/Services and ICC can route
-between their versions. `desk` never derives a version itself -- ICC is the single
-point that mints one.
+`--skew` selects between the two deploy shapes, and naming a `--version` implies
+it. Deploy the same app without either flag and it behaves like any other
+deploy.
+
+With `--skew`, the Deployment and Service are named `{app}-{version}` (e.g.
+`my-watt-project-v1`) and labelled with `app.kubernetes.io/name:
+my-watt-project` and `plt.dev/version: v1`. Each version is a separate workload
+so they coexist while the old one drains, and traffic routes through Gateway API
+HTTPRoutes managed by ICC, which also expires versions once they go idle. Given
+no `--version`, `desk` generates one before the image build so it can be baked in
+as `PLT_DEPLOYMENT_ID`, which is what makes the version pinnable by `?dpl=`.
+
+Without `--skew`, the workload keeps the single name `{app}` and each deploy
+rolls it over in place, replacing the previous pods. No version label is set, so
+ICC records a version for history (deriving a `plt_` id from the image) but
+manages no routing: `desk` writes the HTTPRoute itself. This is the mode to use
+when `skew_protection` is disabled in the profile, since nothing would ever
+remove superseded workloads.
 
 Deploy with a dedicated hostname:
 

cli/deploy.js

@@ -69,7 +69,7 @@ export function imageName (image) {
 
 export default async function cli (argv) {
   const args = minimist(argv, {
-    bool: ['dry-run', 'headless', 'via-icc'],
+    bool: ['dry-run', 'headless', 'via-icc', 'skew'],
     string: [
       'dir',
       'image',
@@ -143,15 +143,22 @@ export default async function cli (argv) {
 
   const isWorkflow = detectWorkflow(dockerfile, envVars)
 
-  // Mint a version when skew protection is on and none was named. It has to be
-  // decided here, before the build, because the id is baked into the client
-  // assets -- a version assigned afterwards is one ICC cannot pin.
+  // --skew is the switch between the two deploy shapes, and naming an explicit
+  // --version implies it. Without it this is an ordinary deploy: one workload
+  // that each deploy replaces, routed by the HTTPRoute desk writes below. With
+  // it, every version is its own workload so they can coexist, and ICC takes
+  // over routing and expiry.
+  const skew = args.skew || !!args.version
+
+  // Mint a version when skew is on and none was named. It has to be decided
+  // here, before the build, because the id is baked into the client assets --
+  // a version assigned afterwards is one ICC cannot pin.
   //
   // Only when building from --dir. A prebuilt --image already carries whatever
   // id it was built with, and a label we invent here would contradict it: ICC
   // would see the mismatch and refuse to route by query anyway.
   const version = args.version ||
-    (directory && skewProtectionEnabled(context) ? generateVersion(appImage) : undefined)
+    (directory && skew ? generateVersion(appImage) : undefined)
   if (version && !args.version) info(`No --version given; using generated version ${version}`)
 
   const buildArgs = deployBuildArgs(version)
@@ -220,8 +227,8 @@ export default async function cli (argv) {
   }
 
   const namespace = args.namespace || 'platformatic'
-  await deploy.createDeployment(appName, appImage, namespace, envVars, args['dry-run'], { context, version, isWorkflow, hostname, minReplicas, maxReplicas })
-  const serviceName = await deploy.createService(appName, appImage, namespace, args['dry-run'], { context, version, isWorkflow, headless: args.headless })
+  await deploy.createDeployment(appName, appImage, namespace, envVars, args['dry-run'], { context, version, skew, isWorkflow, hostname, minReplicas, maxReplicas })
+  const serviceName = await deploy.createService(appName, appImage, namespace, args['dry-run'], { context, version, skew, isWorkflow, headless: args.headless })
 
   if (args.headless) {
     if (!args['dry-run']) {

lib/deploy.js

@@ -1,8 +1,8 @@
 import { spawn } from './utils.js'
 import { addToRun } from './run-directory.js'
 
-// The Deployment/Service name must be unique per version so versions coexist for
-// skew protection. Use the version when it is already a legal name segment;
+// Under skew protection the Deployment/Service name must be unique per version
+// so versions coexist. Use the version when it is already a legal name segment;
 // otherwise fall back to the image tag (unique per build), sanitized. This is
 // only the resource name -- the routing version id is the plt.dev/version label,
 // which keeps the original value.
@@ -19,7 +19,19 @@ export function resourceVersion (version, imageNameTag) {
   return tag.toLowerCase().replace(/[^a-z0-9-]/g, '-').replace(/^-+|-+$/g, '').slice(0, 40) || 'latest'
 }
 
-export async function createDeployment (name, imageNameTag, namespace, envVars, dryRun, { context, version, isWorkflow, hostname, minReplicas, maxReplicas }) {
+// Without skew protection there are no coexisting versions, so the workload
+// keeps one stable name and each deploy rolls it over in place. Suffixing the
+// image tag instead would make every deploy a new Deployment, and nothing would
+// remove the old one: expiry and cleanup are skew machinery, and ICC leaves them
+// off with the feature disabled.
+export function workloadName (name, version, imageNameTag, skew) {
+  // A version implies skew, the same rule the CLI applies to --version, so a
+  // caller that names a version always gets a per-version workload.
+  if (!skew && !version) return name
+  return `${name}-${resourceVersion(version, imageNameTag)}`
+}
+
+export async function createDeployment (name, imageNameTag, namespace, envVars, dryRun, { context, version, skew, isWorkflow, hostname, minReplicas, maxReplicas }) {
   const reservedEnv = new Set(['PLT_INSTANCE_ID', 'PLT_DEPLOYMENT_VERSION', 'PLT_WORLD_APP_ID', 'PLT_WORLD_DEPLOYMENT_VERSION'])
   const defaultResources = {
     // Minimum
@@ -35,9 +47,8 @@ export async function createDeployment (name, imageNameTag, namespace, envVars,
     }
   }
 
-  const rv = resourceVersion(version, imageNameTag)
-  const resourceName = `${name}-${rv}`
-  const instanceLabel = `${name}-${rv}`
+  const resourceName = workloadName(name, version, imageNameTag, skew)
+  const instanceLabel = resourceName
   const nameLabel = name
 
   const labels = {
@@ -166,10 +177,9 @@ export async function createDeployment (name, imageNameTag, namespace, envVars,
   return resourceName
 }
 
-export async function createService (name, imageNameTag, namespace, dryRun, { context, version, isWorkflow, headless }) {
-  const rv = resourceVersion(version, imageNameTag)
-  const resourceName = `${name}-${rv}`
-  const instanceLabel = `${name}-${rv}`
+export async function createService (name, imageNameTag, namespace, dryRun, { context, version, skew, isWorkflow, headless }) {
+  const resourceName = workloadName(name, version, imageNameTag, skew)
+  const instanceLabel = resourceName
   const nameLabel = name
 
   const labels = {

profiles/development.yaml

@@ -50,16 +50,16 @@ platformatic:
         deployer:
           enable: true                     # ICC creates workloads via the deploy API (grants create/patch on Deployments/Services/pull Secrets)
         skew_protection:
-          enable: true
-          auto_cleanup: false              # Keep expired Deployments/Services for inspection
-          http_grace_period_ms: 120000     # 2 min: keep >= traffic_window_ms
-          http_max_alive_ms: 900000         # 15 min for e2e testing
-          workflow_grace_period_ms: 300000  # 5 min for demo
-          workflow_max_alive_ms: 900000    # 15 min for demo
-          check_interval_ms: 10000         # 10 seconds
-          traffic_window_ms: 60000         # 1 min
-          cookie_max_age: 43200            # 12h in seconds (keep default)
-          default_routing_mode: query       # query | cookie; per-app override in ICC settings
+          enable: false
+          # auto_cleanup: false              # Keep expired Deployments/Services for inspection
+          # http_grace_period_ms: 120000     # 2 min: keep >= traffic_window_ms
+          # http_max_alive_ms: 900000         # 15 min for e2e testing
+          # workflow_grace_period_ms: 300000  # 5 min for demo
+          # workflow_max_alive_ms: 900000    # 15 min for demo
+          # check_interval_ms: 10000         # 10 seconds
+          # traffic_window_ms: 60000         # 1 min
+          # cookie_max_age: 43200            # 12h in seconds (keep default)
+          # default_routing_mode: query       # query | cookie; per-app override in ICC settings
         icc_jobs:
           enable: true
 
@@ -95,13 +95,13 @@ platformatic:
 
       log_level: debug
 
-    workflow:
-      hotReload: true
-      localRepo: "{{ WORKFLOW_REPO }}"
-      workingDir: /app/packages/workflow
-
-      replicas:
-        min: 1
-        max: 1
-
-      log_level: info
+    # workflow:
+    #   hotReload: true
+    #   localRepo: "{{ WORKFLOW_REPO }}"
+    #   workingDir: /app/packages/workflow
+    #
+    #   replicas:
+    #     min: 1
+    #     max: 1
+    #
+    #   log_level: info

tests/deploy.test.js

@@ -3,7 +3,7 @@ import { test } from 'node:test'
 import { mkdtemp, readFile, rm } from 'node:fs/promises'
 import { join } from 'node:path'
 import { tmpdir } from 'node:os'
-import { createDeployment, resourceVersion } from '../lib/deploy.js'
+import { createDeployment, resourceVersion, workloadName } from '../lib/deploy.js'
 import { imageName, deployBuildArgs, generateVersion, skewProtectionEnabled } from '../cli/deploy.js'
 
 test('imageName handles registry ports, tags, and digests', () => {
@@ -118,3 +118,53 @@ test('a generated version reaches the image build', () => {
   const version = generateVersion('plt.localreg/plt-local/orders:1786546018096')
   assert.deepEqual(deployBuildArgs(version), { PLT_DEPLOYMENT_ID: version })
 })
+
+test('without skew the workload keeps one name so deploys replace it', async () => {
+  // The bug this fixes: naming from the image tag (a Date.now() value) made every
+  // deploy a new Deployment, and nothing removed the previous one, because expiry
+  // is skew machinery that ICC leaves off with the feature disabled.
+  const first = workloadName('orders', undefined, 'registry/orders:1786613986032', false)
+  const second = workloadName('orders', undefined, 'registry/orders:1786614038051', false)
+
+  assert.equal(first, 'orders')
+  assert.equal(second, first)
+})
+
+test('with skew each version is its own workload', async () => {
+  // Versions have to coexist while the old one drains, so the name must differ.
+  const v1 = workloadName('orders', 'plt_aaaaaaaaaaaaaaaaaaaaaaaa', 'registry/orders:1', true)
+  const v2 = workloadName('orders', 'plt_bbbbbbbbbbbbbbbbbbbbbbbb', 'registry/orders:2', true)
+
+  assert.notEqual(v1, v2)
+  // plt_ ids are not legal name segments, so both fall back to the image tag.
+  assert.equal(v1, 'orders-1')
+  assert.equal(v2, 'orders-2')
+  // An explicit legal version names the workload directly.
+  assert.equal(workloadName('orders', 'v1.2.3', 'registry/orders:9', true), 'orders-v1.2.3')
+})
+
+test('a named version implies a per-version workload', async () => {
+  // The CLI treats --version as implying --skew; the builders must agree, or a
+  // versioned deploy through the library would silently collapse onto one name.
+  assert.equal(workloadName('orders', 'v2', 'registry/orders:9', false), 'orders-v2')
+})
+
+test('an unversioned deploy carries no version label', async () => {
+  const runDir = await mkdtemp(join(tmpdir(), 'desk-deploy-'))
+  try {
+    await createDeployment('orders', 'registry/orders:1786614038051', 'platformatic', {}, true, {
+      context: { runDir },
+      skew: false
+    })
+
+    const manifest = JSON.parse(await readFile(join(runDir, 'deployment.json'), 'utf8'))
+    assert.equal(manifest.metadata.name, 'orders')
+    assert.deepEqual(manifest.metadata.labels, {
+      'app.kubernetes.io/name': 'orders',
+      'app.kubernetes.io/instance': 'orders'
+    })
+    assert.equal(manifest.spec.selector.matchLabels['app.kubernetes.io/instance'], 'orders')
+  } finally {
+    await rm(runDir, { recursive: true, force: true })
+  }
+})