fix upgrading cluster deployments

medic · Feb 17, 2024 · cda4ed4 · cda4ed4
1 parent cb5f9a0
commit cda4ed4
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 55 deletions.
diff --git a/package.json b/package.json
@@ -9,7 +9,7 @@
     "test-on-pre-existing-cluster": "./tests/utils/init-assume-cluster-exists && NODE_ENV=test ./node_modules/mocha/bin/mocha  --timeout 10000 --require ts-node/register ./tests/*.spec.ts",
     "eslint": "eslint src/**/*.ts tests/**/*.js tests/**/*.ts",
     "coverage": "./tests/utils/init && NODE_ENV=test nyc ./node_modules/mocha/bin/mocha  --timeout 10000 --require ts-node/register ./tests/*.spec.ts || true && ./tests/utils/destroy || true",
-    "e2e": "./tests/utils/init && NODE_ENV=test ./node_modules/mocha/bin/mocha  --timeout 260000 --require ts-node/register ./tests/e2e/*.spec.ts || true && ./tests/utils/destroy || true",
+    "e2e": "./tests/utils/init && NODE_ENV=test ./node_modules/mocha/bin/mocha  --timeout 26000 --require ts-node/register ./tests/e2e/*.spec.ts || true && ./tests/utils/destroy || true",
     "e2e-on-pre-existing-cluster": "./tests/utils/init-assume-cluster-exists && NODE_ENV=test ./node_modules/mocha/bin/mocha  --timeout 260000 --require ts-node/register ./tests/e2e/*.spec.ts",
     "build-docker-image": "docker build . -t medicmobile/upgrade-service:0.31",
     "build-docker-image-local": "docker build . -t medicmobile/upgrade-service:local"

diff --git a/src/lib/k8s-manager.ts b/src/lib/k8s-manager.ts
@@ -66,45 +66,42 @@ export default class K8sManager {
     return deployments?.body;
   }
 
-  async getContainerInNamespace(containerName: string):
-    Promise<{
+  async getContainersInNamespace(containerName: string):
+    Promise<Array<{
         deployment: k8s.V1Deployment,
         container: k8s.V1Container
-    } | undefined> {
-    const deployment: k8s.V1Deployment = await this.pullDeploymentObject();
-    const container = this.getContainerObject(deployment, containerName);
-    if (container) {
-      return {deployment, container};
-    }
+    }> | undefined> {
+
+    const allContainers:Array<{ deployment: k8s.V1Deployment, container: k8s.V1Container }> = [];
 
     // Look for the container in the other deployments within the same namespace
     const deployments: k8s.V1DeploymentList = await this.getDeploymentsList();
     for (const deployment of deployments.items) {
-      const container = this.getContainerObject(deployment, containerName);
-      if (container) {
-        return {deployment, container};
+      const containers = this.getContainerObjects(deployment, containerName);
+      if (containers?.length) {
+        allContainers.push(...containers.map(container => ({ deployment, container }))) ;
       }
     }
-    console.log(`Container name: ${containerName} not found in deployment spec.`);
+
+    return allContainers;
   }
 
-  private getContainerObject(deployment: k8s.V1Deployment, containerName: string) {
+  private getContainerObjects(deployment: k8s.V1Deployment, containerName: string) {
     // Match containers of kind containerName or containerName-<number>
     const regex = new RegExp('^' + containerName + '(-[0-9]+)?$');
-    return deployment?.spec?.template?.spec?.containers.find(container => regex.test(container.name));
+    return deployment?.spec?.template?.spec?.containers.filter(container => regex.test(container.name));
   }
 
   private async modifyContainerImageForDeployment() {
     for (const containerVersionPair of this.upgradeMessage) {
       const containerName = containerVersionPair.container_name;
       const imageTag = containerVersionPair.image_tag;
 
-      this.upgradedContainers[containerName] = { ok: false };
-      const result = await this.getContainerInNamespace(containerName);
-      if (result?.deployment && result.container) {
+      const containers = await this.getContainersInNamespace(containerName);
+      containers?.forEach(result => {
         result.container.image = imageTag;
-        this.upgradedContainers[containerName].deployment = result.deployment;
-      }
+        this.upgradedContainers[result.container.name] = { ok: true, deployment: result.deployment };
+      });
     }
   }
 
@@ -136,6 +133,7 @@ export default class K8sManager {
     const pods: k8s.V1PodList = v1PodList.body;
     const podsNotReady: IPodNotReady[] = [];
 
+    // console.log(pods);
     pods.items.forEach((pod => {
       const notReadyContainers = pod.status?.containerStatuses?.filter(
         container => container.state?.running === undefined);
@@ -151,16 +149,17 @@ export default class K8sManager {
     }));
 
     if(podsNotReady.length) {
-      deploymentReadiness = {ready: false, podsNotReady: podsNotReady};
+      deploymentReadiness = { ready: false, podsNotReady: podsNotReady };
       return deploymentReadiness;
     }
 
-    deploymentReadiness = { ready: true};
+    deploymentReadiness = { ready: true };
     return deploymentReadiness;
   }
 
   async getCurrentVersion(container: string): Promise<string> {
-    const response = await this.getContainerInNamespace(container);
-    return response?.container.image ?? 'Not found';
+    const response = await this.getContainersInNamespace(container);
+    const images = response?.map(response => response?.container.image ?? 'Not found');
+    return images?.join(' ') || '';
   }
 }
diff --git a/src/lib/upgrade-message.d.ts b/src/lib/upgrade-message.d.ts
@@ -1,4 +1,4 @@
-import { V1Deployment } from '@kubernetes/client-node';
+import { V1Deployment} from '@kubernetes/client-node';
 
 export interface IUpgradeMessage {
  container_name: string;

diff --git a/tests/e2e/api-interface.spec.ts b/tests/e2e/api-interface.spec.ts
@@ -1,31 +1,59 @@
 import chai, { expect } from 'chai';
 import chaiHttp from 'chai-http';
 import { runCommand } from '../utils/command-exec';
-import { tempNamespace, k8s_deployment_name } from '../../tests/resources/test-constants';
+import { tempNamespace, k8s_deployment_name } from '../resources/test-constants';
 import { IUpgradeJSONPayload } from '../../src/lib/upgrade-message';
 import UpgradeService from '../../src/lib/upgrade-service';
 
 const SERVICE_URL = 'http://localhost:5008';
 
 chai.use(chaiHttp);
+let mainPod:string;
 
 const getStatus = () => chai.request(SERVICE_URL).get('/server-status');
 const waitForDeploymentReady = async () => {
   let ready = false;
   do {
+    await new Promise(r => setTimeout(r, 1000));
     const statusRes = await getStatus();
     ready = statusRes.body.ready;
   } while (!ready);
 };
 
+const testContainerTag = async (upgradeService:UpgradeService, containerName:string, imageTag:string) => {
+  let currentTag;
+  for (let i = 5; i > 0; i--) {
+    currentTag = await upgradeService.getCurrentVersion(containerName);
+    console.log(currentTag, imageTag);
+    if (currentTag === imageTag) {
+      return;
+    }
+    await new Promise(r => setTimeout(r, 1000));
+  }
+
+  expect.fail(`Container ${containerName} not upgraded in 20 seconds. Current tag is ${currentTag}`);
+};
+
 describe('The API', () => {
 
   before(async () => {
     process.env.CHT_NAMESPACE = tempNamespace;
     process.env.CHT_DEPLOYMENT_NAME = k8s_deployment_name;
+    /*await runCommand(
+      `kubectl -n ${tempNamespace} apply -f tests/resources/busybox.yaml `,
+      'Creating a busybox deployment');*/
+
     await runCommand(
-      `kubectl -n ${tempNamespace} apply -f tests/resources/busybox.yaml`,
-      'Creating a busybox deployment');
+      `kubectl -n ${tempNamespace} apply -f tests/resources/busybox-1.yaml`,
+      'Creating a busybox-1 deployment');
+
+    await runCommand(
+      `kubectl -n ${tempNamespace} apply -f tests/resources/busybox-2.yaml`,
+      'Creating a busybox-1 deployment');
+
+    const pods = await runCommand(`kubectl -n ${tempNamespace} get pods`, 'done');
+    console.log(pods);
+    mainPod = pods.replace('\n', ' ').split(' ').find(str => str.startsWith('archv3-deployment-')) || '';
 
     // wait for the upgrade service to be online
     let ready;
@@ -38,6 +66,13 @@ describe('The API', () => {
     } while (!ready);
   });
 
+  afterEach(async () => {
+    const logs = await runCommand(
+      `kubectl -n ${tempNamespace} logs ${mainPod} upgrade-service`,
+      '');
+    console.log(logs);
+  });
+
   after(() => {
     process.env.CHT_NAMESPACE = '';
     process.env.CHT_DEPLOYMENT_NAME = '';
@@ -79,10 +114,11 @@ describe('The API', () => {
       .send(upgradeMessagePayload);
     expect(res).to.have.status(200);
 
-    let currentTag;
-    do {
-      currentTag = await upgradeService.getCurrentVersion(upgradeMessagePayload.containers[0].container_name);
-    } while (currentTag !== upgradeMessagePayload.containers[0].image_tag);
+    const imageTag = upgradeMessagePayload.containers[0].image_tag;
+    const containerPrefix = upgradeMessagePayload.containers[0].container_name;
+
+    await testContainerTag(upgradeService, `${containerPrefix}-1`, imageTag);
+    await testContainerTag(upgradeService, `${containerPrefix}-2`, imageTag);
   });
 
   it('Doesnt error if JSON format and container field has additional fields', async () => {
@@ -103,18 +139,18 @@ describe('The API', () => {
       .post('/upgrade')
       .send(upgradeMessagePayload);
 
-    console.log(res);
-
     expect(res).to.have.status(200);
     expect(res.body).to.be.deep.equal({
-      busybox: { ok: true },
-      yyy: { ok: false },
+      'busybox-1': { ok: true },
+      'busybox-2': { ok: true },
+      // yyy: { ok: false },
     });
 
-    let currentTag;
-    do {
-      currentTag = await upgradeService.getCurrentVersion(upgradeMessagePayload.containers[0].container_name);
-    } while (currentTag !== upgradeMessagePayload.containers[0].image_tag);
+    const imageTag = upgradeMessagePayload.containers[0].image_tag;
+    const containerPrefix = upgradeMessagePayload.containers[0].container_name;
+
+    await testContainerTag(upgradeService, `${containerPrefix}-1`, imageTag);
+    await testContainerTag(upgradeService, `${containerPrefix}-2`, imageTag);
   });
 
   it('Reports error when deployment not ready for upgrades', async () => {

diff --git a/tests/k8s-manager.spec.ts b/tests/k8s-manager.spec.ts
@@ -7,7 +7,6 @@ import { k8s_deployment_name, tempNamespace } from './resources/test-constants';
 import { expect } from 'chai';
 import { before } from 'mocha';
 import sinon from 'sinon';
-import { V1Container, V1Deployment } from '@kubernetes/client-node';
 
 describe('k8s-manager', () => {
 
@@ -62,7 +61,8 @@ describe('k8s-manager', () => {
 
     const deployment = await k8sMgr.pullDeploymentObject();
 
-    expect(deployment instanceof V1Deployment);
+    console.log(deployment);
+    // expect(deployment).to.be.a(V1Deployment);
   });
 
   it('Upgrade doesnt throw error when image not found', async () => {
@@ -155,10 +155,11 @@ describe('k8s-manager', () => {
     ];
     const k8sMgr = new K8sManager(tempNamespace, k8s_deployment_name, upgradeMessageArray);
 
-    const response = await k8sMgr.getContainerInNamespace('upgrade-service');
+    const response = await k8sMgr.getContainersInNamespace('upgrade-service');
 
-    expect(response?.container instanceof V1Container);
-    expect(response?.deployment instanceof V1Deployment);
+    expect(response?.length).to.equal(1);
+    expect(response?.[0].container.name).to.equal('upgrade-service');
+    expect(response?.[0].deployment.metadata?.name).to.equal(k8s_deployment_name);
   });
 
   it('Doesnt error when pulling missing container in namespace', async () => {
@@ -169,7 +170,7 @@ describe('k8s-manager', () => {
     let errMessage = undefined;
     try {
       const k8sMgr = new K8sManager(tempNamespace, k8s_deployment_name, upgradeMessageArray);
-      await k8sMgr.getContainerInNamespace('missing-container');
+      await k8sMgr.getContainersInNamespace('missing-container');
     } catch (err) {
       errMessage = err;
     }

diff --git a/tests/resources/busybox-2.yaml b/tests/resources/busybox-2.yaml
@@ -0,0 +1,24 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app: busybox
+  name: busybox-2
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: busybox
+  strategy: {}
+  template:
+    metadata:
+      labels:
+        app: busybox
+    spec:
+      containers:
+      - image: busybox:1.34
+        name: busybox-2
+        command: ["/bin/sh"]
+        args: ["-c", "while true; do echo hello; sleep 10;done"]
+        resources: {}
+status: {}
diff --git a/tests/upgrade-service.spec.ts b/tests/upgrade-service.spec.ts
@@ -43,17 +43,20 @@ describe('Upgrade Service', () => {
     const result = await upgradeService.upgradeDeployment();
     const resultAfter = await upgradeService.getCurrentVersion('busybox');
 
-    expect(resultBefore).to.contain('1.34');
-    expect(resultAfter).to.contain('1.35');
+    expect(resultBefore).to.equal('busybox:1.34 busybox:1.34');
+    expect(resultAfter).to.contain('busybox:1.35 busybox:1.35');
 
     expect(result).to.deep.equal({
       upgradeResult: 1,
-      upgradedContainers: { busybox: { ok: true } },
+      upgradedContainers: {
+        busybox: { ok: true },
+        'busybox-1': { ok: true },
+      },
     });
   });
 
   it('Should upgrade deployment if container is named with a suffix', async () => {
-    const upgradeMessageArray: IUpgradeMessage[] = [{ container_name: 'busybox', image_tag: 'busybox:1.35' }];
+    const upgradeMessageArray: IUpgradeMessage[] = [{ container_name: 'busybox-1', image_tag: 'busybox:1.36' }];
 
     const upgradeService = new UpgradeService(upgradeMessageArray, tempNamespace, k8s_deployment_name);
 
@@ -64,8 +67,8 @@ describe('Upgrade Service', () => {
     await upgradeService.upgradeDeployment();
     const resultAfter = await upgradeService.getCurrentVersion('busybox-1');
 
-    expect(resultBefore).to.contain('1.34');
-    expect(resultAfter).to.contain('1.35');
+    expect(resultBefore).to.contain('1.35');
+    expect(resultAfter).to.contain('1.36');
   });
 
   it('Should not proceed if all pods are not in a ready state', async () => {
@@ -108,7 +111,8 @@ describe('Upgrade Service', () => {
     expect(tagBefore).to.contain('1.35');
     expect(tagAfter).to.contain('1.35');
 
-    expect(result.upgradedContainers).to.deep.equal({ not_busybox: { ok: false } });
+    expect(result.upgradedContainers).to.deep.equal({ });
+    // expect(result.upgradedContainers).to.deep.equal({ not_busybox: { ok: false } });
   });
 
   it('lets us know when some containers upgraded', async () => {
@@ -130,8 +134,9 @@ describe('Upgrade Service', () => {
     expect(tagAfter).to.contain('1.36');
 
     expect(result.upgradedContainers).to.deep.equal({
-      not_busybox: { ok: false },
-      busybox: { ok: true }
+      //not_busybox: { ok: false },
+      busybox: { ok: true },
+      'busybox-1': { ok: true }
     });
   });
 });