diff --git a/README.md b/README.md index bf6edf1..5038a09 100644 --- a/README.md +++ b/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: diff --git a/cli/deploy.js b/cli/deploy.js index 2540582..7cf1b16 100644 --- a/cli/deploy.js +++ b/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']) { diff --git a/lib/deploy.js b/lib/deploy.js index c23ef76..bde8345 100644 --- a/lib/deploy.js +++ b/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 = { diff --git a/profiles/development.yaml b/profiles/development.yaml index 298f432..5bf5f33 100644 --- a/profiles/development.yaml +++ b/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 diff --git a/tests/deploy.test.js b/tests/deploy.test.js index 49d77be..14de417 100644 --- a/tests/deploy.test.js +++ b/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 }) + } +})