fix: do not perform runtimeUpgrades if deployingVersion and version are the same (#2742)

Author: j-zimnowoda

package.json

@@ -163,9 +163,9 @@
     "validate-templates": "ENV_DIR=$PWD/tests/fixtures NODE_ENV=test binzx/otomi validate-templates",
     "validate-templates:all": "set -e; i=31; while [ $i -le 34 ]; do NODE_ENV=test binzx/otomi validate-templates -k 1.$i; i=$(($i+1)); done",
     "validate-values": "ENV_DIR=$PWD/tests/fixtures NODE_ENV=test binzx/otomi validate-values",
-    "bootstrap-dev": "rm -rf /tmp/otomi-bootstrap-dev; CI=1 VALUES_INPUT=$PWD/tests/bootstrap/input-local-dev.yaml ENV_DIR=/tmp/otomi-bootstrap-dev binzx/otomi bootstrap",
-    "bootstrap-dev-with-repo": "CI=1 ENV_DIR=/tmp/otomi-bootstrap-dev binzx/otomi bootstrap",
-    "bootstrap-tests-fixtures": "CI=1 ENV_DIR=$PWD/tests/fixtures binzx/otomi bootstrap"
+    "bootstrap-dev": "rm -rf /tmp/otomi-bootstrap-dev; VALUES_INPUT=$PWD/tests/bootstrap/input-local-dev.yaml ENV_DIR=/tmp/otomi-bootstrap-dev binzx/otomi bootstrap",
+    "bootstrap-dev-with-repo": "ENV_DIR=/tmp/otomi-bootstrap-dev binzx/otomi bootstrap",
+    "bootstrap-tests-fixtures": "ENV_DIR=$PWD/tests/fixtures binzx/otomi bootstrap"
   },
   "standard-version": {
     "skip": {

src/cmd/apply-as-apps.ts

@@ -7,12 +7,12 @@ import { logLevelString, terminal } from 'src/common/debug'
 import { hf } from 'src/common/hf'
 import { appRevisionMatches, k8s, patchArgoCdApp, patchContainerResourcesOfSts } from 'src/common/k8s'
 import { getFilename, loadYaml } from 'src/common/utils'
-import { getImageTag, objectToYaml } from 'src/common/values'
+import { getImageTagFromValues, objectToYaml } from 'src/common/values'
 import { getParsedArgs, HelmArguments, helmOptions, setParsedArgs } from 'src/common/yargs'
 import { Argv, CommandModule } from 'yargs'
 import { $ } from 'zx'
-import { env } from '../common/envalid'
 import { ARGOCD_APP_DEFAULT_SYNC_POLICY } from '../common/constants'
+import { env } from '../common/envalid'
 
 const cmdName = getFilename(__filename)
 const dir = '/tmp/otomi'
@@ -171,7 +171,7 @@ export const applyAsApps = async (argv: HelmArguments): Promise<boolean> => {
   const helmfileSource = argv.file?.toString() || 'helmfile.d/'
   d.info(`Parsing helm releases defined in ${helmfileSource}`)
   setup()
-  const otomiVersion = await getImageTag()
+  const otomiVersion = await getImageTagFromValues()
   try {
     const expectedRevision = env.APPS_REVISION || otomiVersion
     d.info('Checking running revision of apl-operator...')

src/cmd/apply.test.ts

@@ -20,8 +20,8 @@ jest.mock('src/common/k8s', () => ({
 }))
 
 jest.mock('src/common/values', () => ({
-  getCurrentVersion: jest.fn(),
-  getImageTag: jest.fn(),
+  getImageTagFromValues: jest.fn(),
+  getPackageVersion: jest.fn(),
 }))
 
 jest.mock('zx', () => ({
@@ -40,10 +40,6 @@ jest.mock('./commit', () => ({
   commit: jest.fn(),
 }))
 
-jest.mock('./upgrade', () => ({
-  upgrade: jest.fn(),
-}))
-
 jest.mock('src/common/runtime-upgrade', () => ({
   runtimeUpgrade: jest.fn(),
 }))
@@ -92,23 +88,21 @@ describe('Apply command', () => {
     mockDeps = {
       getDeploymentState: require('src/common/k8s').getDeploymentState,
       setDeploymentState: require('src/common/k8s').setDeploymentState,
-      getCurrentVersion: require('src/common/values').getCurrentVersion,
-      getImageTag: require('src/common/values').getImageTag,
+      getImageTagFromValues: require('src/common/values').getImageTagFromValues,
+      getPackageVersion: require('src/common/values').getPackageVersion,
       applyAsApps: require('./apply-as-apps').applyAsApps,
       commit: require('./commit').commit,
-      upgrade: require('./upgrade').upgrade,
       runtimeUpgrade: require('src/common/runtime-upgrade').runtimeUpgrade,
       cd: require('zx').cd,
       getParsedArgs: require('src/common/yargs').getParsedArgs,
     }
 
     // Set up default mock return values
     mockDeps.getDeploymentState.mockResolvedValue({ status: 'deployed' })
-    mockDeps.getCurrentVersion.mockResolvedValue('1.0.0')
-    mockDeps.getImageTag.mockResolvedValue('v1.0.0')
+    mockDeps.getImageTagFromValues.mockResolvedValue('v1.0.0')
+    mockDeps.getPackageVersion.mockReturnValue('1.0.0')
     mockDeps.applyAsApps.mockResolvedValue(true)
     mockDeps.commit.mockResolvedValue(undefined)
-    mockDeps.upgrade.mockResolvedValue(undefined)
     mockDeps.runtimeUpgrade.mockResolvedValue(undefined)
     mockDeps.getParsedArgs.mockReturnValue({})
   })
@@ -151,13 +145,11 @@ describe('Apply command', () => {
       await applyAll()
 
       // Verify pre-upgrade steps
-      expect(mockDeps.upgrade).toHaveBeenCalledWith({ when: 'pre' })
       expect(mockDeps.runtimeUpgrade).toHaveBeenCalledWith({ when: 'pre' })
 
       // Verify deployment state management
       expect(mockDeps.getDeploymentState).toHaveBeenCalled()
-      expect(mockDeps.getImageTag).toHaveBeenCalled()
-      expect(mockDeps.getCurrentVersion).toHaveBeenCalled()
+      expect(mockDeps.getImageTagFromValues).toHaveBeenCalled()
       expect(mockDeps.setDeploymentState).toHaveBeenCalledWith({
         status: 'deploying',
         deployingTag: 'v1.0.0',
@@ -168,7 +160,6 @@ describe('Apply command', () => {
       expect(mockDeps.applyAsApps).toHaveBeenCalled()
 
       // Verify post-upgrade steps
-      expect(mockDeps.upgrade).toHaveBeenCalledWith({ when: 'post' })
       expect(mockDeps.runtimeUpgrade).toHaveBeenCalledWith({ when: 'post' })
 
       // Verify final state
@@ -260,7 +251,6 @@ describe('Apply command', () => {
 
       expect(mockDeps.applyAsApps).toHaveBeenCalledWith(testArgs)
       // Should not go through the full applyAll flow
-      expect(mockDeps.upgrade).not.toHaveBeenCalled()
     })
 
     test('should call applyAsApps directly when file specified', async () => {
@@ -271,7 +261,6 @@ describe('Apply command', () => {
 
       expect(mockDeps.applyAsApps).toHaveBeenCalledWith(testArgs)
       // Should not go through the full applyAll flow
-      expect(mockDeps.upgrade).not.toHaveBeenCalled()
     })
 
     test('should call applyAsApps directly when both label and file specified', async () => {
@@ -282,7 +271,6 @@ describe('Apply command', () => {
 
       expect(mockDeps.applyAsApps).toHaveBeenCalledWith(testArgs)
       // Should not go through the full applyAll flow
-      expect(mockDeps.upgrade).not.toHaveBeenCalled()
     })
 
     test('should handle retry logic when applyAll fails', async () => {
@@ -310,7 +298,7 @@ describe('Apply command', () => {
 
     test('should handle image tag retrieval errors', async () => {
       const error = new Error('Failed to get image tag')
-      mockDeps.getImageTag.mockRejectedValueOnce(error)
+      mockDeps.getImageTagFromValues.mockRejectedValueOnce(error)
 
       await expect(applyAll()).rejects.toThrow('Failed to get image tag')
     })
@@ -322,13 +310,6 @@ describe('Apply command', () => {
       await expect(applyAll()).rejects.toThrow('ApplyAsApps failed')
     })
 
-    test('should handle upgrade errors', async () => {
-      const error = new Error('Pre-upgrade failed')
-      mockDeps.upgrade.mockRejectedValueOnce(error)
-
-      await expect(applyAll()).rejects.toThrow('Pre-upgrade failed')
-    })
-
     test('should handle runtime upgrade errors', async () => {
       const error = new Error('Runtime upgrade failed')
       mockDeps.runtimeUpgrade.mockRejectedValueOnce(error)

src/cmd/apply.ts

@@ -6,7 +6,7 @@ import { terminal } from 'src/common/debug'
 import { env } from 'src/common/envalid'
 import { getDeploymentState, setDeploymentState } from 'src/common/k8s'
 import { getFilename, rootDir } from 'src/common/utils'
-import { getCurrentVersion, getImageTag } from 'src/common/values'
+import { getImageTagFromValues, getPackageVersion } from 'src/common/values'
 import { getParsedArgs, HelmArguments, helmOptions, setParsedArgs } from 'src/common/yargs'
 import { Argv, CommandModule } from 'yargs'
 import { cd } from 'zx'
@@ -15,7 +15,6 @@ import { applyAsApps } from './apply-as-apps'
 import { applyTeams } from './apply-teams'
 import { commit } from './commit'
 import { collectTraces } from './traces'
-import { upgrade } from './upgrade'
 
 const cmdName = getFilename(__filename)
 const dir = '/tmp/otomi/'
@@ -36,14 +35,13 @@ export const applyAll = async () => {
   const prevState = await getDeploymentState()
   const argv: HelmArguments = getParsedArgs()
 
-  await upgrade({ when: 'pre' })
   await runtimeUpgrade({ when: 'pre' })
 
   d.info('Start apply all')
   d.info(`Deployment state: ${JSON.stringify(prevState)}`)
-  const tag = await getImageTag()
-  const version = await getCurrentVersion()
-  await setDeploymentState({ status: 'deploying', deployingTag: tag, deployingVersion: version })
+  const tag = await getImageTagFromValues()
+  const deployingVersion = getPackageVersion()
+  await setDeploymentState({ status: 'deploying', deployingTag: tag, deployingVersion })
 
   // We still need to deploy all teams because some settings depend on platform apps.
   const teamsApplyCompleted = await applyTeams()
@@ -55,7 +53,6 @@ export const applyAll = async () => {
   const appsApplyCompleted = await applyAsApps(params)
 
   if (appsApplyCompleted) {
-    await upgrade({ when: 'post' })
     await runtimeUpgrade({ when: 'post' })
   } else {
     d.info('Apps apply step not completed, skipping upgrade checks')
@@ -65,7 +62,7 @@ export const applyAll = async () => {
     await commit(false)
   }
   if (appsApplyCompleted) {
-    await setDeploymentState({ status: 'deployed', version })
+    await setDeploymentState({ status: 'deployed', version: deployingVersion })
     d.info('Deployment completed')
   } else {
     d.info('Deployment finished with actions pending')

src/cmd/bootstrap.test.ts

@@ -1,4 +1,5 @@
 import { cloneDeep, merge } from 'lodash'
+import { pki } from 'node-forge'
 import { env } from 'process'
 import stubs from 'src/test-stubs'
 import {
@@ -11,7 +12,6 @@ import {
   handleFileEntry,
   processValues,
 } from './bootstrap'
-import { pki } from 'node-forge'
 
 const { terminal } = stubs
 
@@ -39,9 +39,8 @@ describe('Bootstrapping values', () => {
       encrypt: jest.fn(),
       existsSync: jest.fn(),
       genSops: jest.fn(),
-      getCurrentVersion: jest.fn(),
       getDeploymentState: jest.fn().mockReturnValue({}),
-      getImageTag: jest.fn(),
+      getImageTagFromValues: jest.fn(),
       getK8sSecret: jest.fn(),
       hfValues: jest.fn(),
       isCli: true,
@@ -59,7 +58,6 @@ describe('Bootstrapping values', () => {
     await bootstrap(deps)
     expect(deps.copyBasicFiles).toHaveBeenCalled()
     expect(deps.bootstrapSops).toHaveBeenCalled()
-    expect(deps.getImageTag).toHaveBeenCalled()
   })
   it('should copy only skeleton files to env dir if it is empty or nonexisting', async () => {
     deps.processValues.mockReturnValue(undefined)

src/cmd/bootstrap.ts

@@ -10,12 +10,12 @@ import { prepareEnvironment } from 'src/common/cli'
 import { DEPLOYMENT_PASSWORDS_SECRET } from 'src/common/constants'
 import { decrypt, encrypt } from 'src/common/crypt'
 import { terminal } from 'src/common/debug'
-import { env, isChart, isCi, isCli } from 'src/common/envalid'
+import { env, isChart, isCli } from 'src/common/envalid'
 import { hfValues } from 'src/common/hf'
 import { createK8sSecret, getDeploymentState, getK8sSecret, secretId } from 'src/common/k8s'
 import { getKmsSettings } from 'src/common/repo'
 import { ensureTeamGitOpsDirectories, getFilename, gucci, isCore, loadYaml, rootDir } from 'src/common/utils'
-import { generateSecrets, getCurrentVersion, getImageTag, writeValues } from 'src/common/values'
+import { generateSecrets, writeValues } from 'src/common/values'
 import { BasicArguments, setParsedArgs } from 'src/common/yargs'
 import { Argv } from 'yargs'
 import { $ } from 'zx'
@@ -292,7 +292,7 @@ export const processValues = async (
     addInitialPasswords,
     addPlatformAdmin,
   },
-): Promise<Record<string, any> | undefined> => {
+): Promise<Record<string, any>> => {
   const d = deps.terminal(`cmd:${cmdName}:processValues`)
   const { ENV_DIR, VALUES_INPUT } = env
   let originalInput: Record<string, any> = {}
@@ -436,8 +436,6 @@ export const bootstrap = async (
   deps = {
     pathExists: existsSync,
     getDeploymentState,
-    getImageTag,
-    getCurrentVersion,
     terminal,
     copyBasicFiles,
     processValues,
@@ -453,37 +451,14 @@ export const bootstrap = async (
   },
 ): Promise<void> => {
   const d = deps.terminal(`cmd:${cmdName}:bootstrap`)
-
-  // if CI: we are called from pipeline on each deployment, which is costly
-  // so run bootstrap only when no previous deployment was done or version or tag of otomi changed
-  const tag = await deps.getImageTag()
-  const version = await deps.getCurrentVersion()
-  if (isCi) {
-    const { version: prevVersion, tag: prevTag } = await deps.getDeploymentState()
-    if (prevVersion && prevTag && version === prevVersion && tag === prevTag) return
-  }
   const { ENV_DIR } = env
-  const hasOtomi = deps.pathExists(`${ENV_DIR}/bin/otomi`)
 
-  const otomiImage = `linode/apl-core:${tag}`
-  d.log(`Installing artifacts from ${otomiImage}`)
   await deps.copyBasicFiles()
   await deps.migrate()
   const originalValues = await deps.processValues()
-  // exit early if `isCli` and `ENV_DIR` were empty, and let the user provide valid values first:
-
-  if (!originalValues) {
-    // FIXME what is the use case to enter this
-    d.log('A new values repo has been created. For next steps follow documentation at https://apl-docs.net')
-    return
-  }
-
   await deps.handleFileEntry()
   await deps.bootstrapSops()
   await ensureTeamGitOpsDirectories(ENV_DIR, originalValues)
-  if (!hasOtomi) {
-    d.log('You can now use the otomi CLI')
-  }
   d.log(`Done bootstrapping values`)
 }
 

src/cmd/commit.ts

@@ -1,16 +1,14 @@
 import retry from 'async-retry'
-import { existsSync } from 'fs'
-import { rm } from 'fs/promises'
 import { bootstrapGit, setIdentity } from 'src/common/bootstrap'
 import { prepareEnvironment } from 'src/common/cli'
 import { encrypt } from 'src/common/crypt'
 import { terminal } from 'src/common/debug'
-import { env, isCi } from 'src/common/envalid'
+import { env } from 'src/common/envalid'
 import { hfValues } from 'src/common/hf'
 import { createUpdateConfigMap, createUpdateGenericSecret, k8s, waitTillGitRepoAvailable } from 'src/common/k8s'
 import { getFilename, loadYaml } from 'src/common/utils'
 import { getRepo } from 'src/common/values'
-import { getParsedArgs, HelmArguments, setParsedArgs } from 'src/common/yargs'
+import { HelmArguments, setParsedArgs } from 'src/common/yargs'
 import { Argv } from 'yargs'
 import { $, cd } from 'zx'
 import { validateValues } from './validate-values'
@@ -55,12 +53,10 @@ const cleanupGitState = async (d: any): Promise<void> => {
   }
 }
 
-const commitAndPush = async (values: Record<string, any>, branch: string): Promise<void> => {
+const commitAndPush = async (values: Record<string, any>, branch: string, initialInstall = false): Promise<void> => {
   const d = terminal(`cmd:${cmdName}:commitAndPush`)
   d.info('Committing values')
-  const argv = getParsedArgs()
-  const rerunRequested = existsSync(`${env.ENV_DIR}/.rerun`)
-  const message = isCi ? 'updated values [ci skip]' : argv.message || 'otomi commit'
+  const message = initialInstall ? 'otomi commit' : 'updated values [ci skip]'
   const { password } = getRepo(values)
   cd(env.ENV_DIR)
   try {
@@ -73,18 +69,14 @@ const commitAndPush = async (values: Record<string, any>, branch: string): Promi
       await $`git commit -m ${message} --no-verify`.quiet()
     }
     await $`git add -A`
-    if (isCi && rerunRequested) {
-      d.log('Committing changes and triggering pipeline run')
-      await $`git commit -m "[apl-trigger]" --no-verify --allow-empty`.quiet()
-    } else {
-      // The below 'git status' command will always return at least single new line
-      const filesChangedCount = (await $`git status --untracked-files=no --porcelain`).toString().split('\n').length - 1
-      if (filesChangedCount === 0) {
-        d.log('Nothing to commit')
-        return
-      }
-      await $`git commit -m ${message} --no-verify`.quiet()
+
+    // The below 'git status' command will always return at least single new line
+    const filesChangedCount = (await $`git status --untracked-files=no --porcelain`).toString().split('\n').length - 1
+    if (filesChangedCount === 0) {
+      d.log('Nothing to commit')
+      return
     }
+    await $`git commit -m ${message} --no-verify`.quiet()
   } catch (e) {
     d.log('commitAndPush error ', e?.message?.replace(password, '****'))
     return
@@ -132,9 +124,6 @@ const commitAndPush = async (values: Record<string, any>, branch: string): Promi
       maxTimeout: 30000,
     },
   )
-  if (rerunRequested) {
-    await rm(`${env.ENV_DIR}/.rerun`, { force: true })
-  }
   d.log('Successfully pushed the updated values')
 }
 
@@ -159,7 +148,7 @@ export const commit = async (initialInstall: boolean, overrideArgs?: HelmArgumen
   }
   // continue
   await encrypt()
-  await commitAndPush(values, branch)
+  await commitAndPush(values, branch, initialInstall)
 }
 
 export const cloneOtomiChartsInGitea = async (): Promise<void> => {