From 9f7d28b9ac644e73816c29f6d6683ef3d37f4c57 Mon Sep 17 00:00:00 2001 From: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com> Date: Tue, 19 Nov 2024 15:23:28 -0800 Subject: [PATCH 1/5] adding timeout to the rollout status --- src/types/kubectl.test.ts | 41 +++++++++++++++++++++++++ src/types/kubectl.ts | 13 +++++--- src/utilities/manifestStabilityUtils.ts | 6 ++-- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/types/kubectl.test.ts b/src/types/kubectl.test.ts index a8ff9f423..91cbf6479 100644 --- a/src/types/kubectl.test.ts +++ b/src/types/kubectl.test.ts @@ -371,6 +371,7 @@ describe('Kubectl class', () => { it('checks rollout status', async () => { const resourceType = 'type' const name = 'name' + const timeout = '60s' expect(await kubectl.checkRolloutStatus(resourceType, name)).toBe( execReturn ) @@ -399,6 +400,46 @@ describe('Kubectl class', () => { ], {silent: false} ) + + // with timeout + await kubectl.checkRolloutStatus( + resourceType, + name, + testNamespace, + timeout + ) + expect(exec.getExecOutput).toBeCalledWith( + kubectlPath, + [ + 'rollout', + 'status', + `${resourceType}/${name}`, + '--namespace', + testNamespace, + `--timeout=${timeout}` + ], + {silent: false} + ) + + // overridden ns and timeout + await kubectl.checkRolloutStatus( + resourceType, + name, + otherNamespace, + timeout + ) + expect(exec.getExecOutput).toBeCalledWith( + kubectlPath, + [ + 'rollout', + 'status', + `${resourceType}/${name}`, + '--namespace', + otherNamespace, + `--timeout=${timeout}` + ], + {silent: false} + ) }) it('gets resource', async () => { diff --git a/src/types/kubectl.ts b/src/types/kubectl.ts index c096c77e1..dcc080645 100644 --- a/src/types/kubectl.ts +++ b/src/types/kubectl.ts @@ -157,13 +157,16 @@ export class Kubectl { public async checkRolloutStatus( resourceType: string, name: string, - namespace?: string + namespace?: string, + timeout?: string ): Promise { - return await this.execute( - ['rollout', 'status', `${resourceType}/${name}`].concat( - this.getFlags(namespace) - ) + const command = ['rollout', 'status', `${resourceType}/${name}`].concat( + this.getFlags(namespace) ) + if (timeout) { + command.push(`--timeout=${timeout}`) + } + return await this.execute(command) } public async getResource( diff --git a/src/utilities/manifestStabilityUtils.ts b/src/utilities/manifestStabilityUtils.ts index cd4b1390b..0c2700520 100644 --- a/src/utilities/manifestStabilityUtils.ts +++ b/src/utilities/manifestStabilityUtils.ts @@ -9,7 +9,8 @@ const POD = 'pod' export async function checkManifestStability( kubectl: Kubectl, - resources: Resource[] + resources: Resource[], + timeout?: string ): Promise { let rolloutStatusHasErrors = false for (let i = 0; i < resources.length; i++) { @@ -24,7 +25,8 @@ export async function checkManifestStability( const result = await kubectl.checkRolloutStatus( resource.type, resource.name, - resource.namespace + resource.namespace, + timeout ) checkForErrors([result]) } catch (ex) { From df15485ccf0681aa43e99333bae9cdcb0b5fd25c Mon Sep 17 00:00:00 2001 From: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com> Date: Fri, 22 Nov 2024 11:50:46 -0800 Subject: [PATCH 2/5] adding timeout in action.yaml and in other functions --- action.yml | 4 + src/actions/deploy.ts | 5 +- src/actions/promote.ts | 34 ++++++-- src/actions/reject.ts | 31 ++++--- src/run.ts | 12 +-- .../blueGreen/blueGreenHelper.test.ts | 27 +++++- .../blueGreen/blueGreenHelper.ts | 13 ++- src/strategyHelpers/blueGreen/reject.test.ts | 84 +++++++++++++++++++ src/strategyHelpers/blueGreen/reject.ts | 30 ++++--- src/strategyHelpers/blueGreen/route.ts | 18 ++-- .../blueGreen/smiBlueGreenHelper.test.ts | 24 +++++- .../blueGreen/smiBlueGreenHelper.ts | 10 ++- src/strategyHelpers/canary/canaryHelper.ts | 16 ++-- src/strategyHelpers/canary/podCanaryHelper.ts | 6 +- src/strategyHelpers/canary/smiCanaryHelper.ts | 40 ++++++--- src/strategyHelpers/deploymentHelper.ts | 9 +- src/types/kubectl.test.ts | 60 ++++++++----- src/types/kubectl.ts | 49 +++++++++-- 18 files changed, 368 insertions(+), 104 deletions(-) diff --git a/action.yml b/action.yml index e391809c8..a68e11109 100644 --- a/action.yml +++ b/action.yml @@ -80,6 +80,10 @@ inputs: skip-tls-verify: description: True if the insecure-skip-tls-verify option should be used. Input should be 'true' or 'false'. default: false + timeout: + description: 'Timeout for the rollout status' + required: false + default: 10m branding: color: 'green' diff --git a/src/actions/deploy.ts b/src/actions/deploy.ts index 17311298c..791aff83e 100644 --- a/src/actions/deploy.ts +++ b/src/actions/deploy.ts @@ -17,7 +17,8 @@ import {parseTrafficSplitMethod} from '../types/trafficSplitMethod' export async function deploy( kubectl: Kubectl, manifestFilePaths: string[], - deploymentStrategy: DeploymentStrategy + deploymentStrategy: DeploymentStrategy, + timeout?: string ) { // update manifests const inputManifestFiles: string[] = updateManifestFiles(manifestFilePaths) @@ -45,7 +46,7 @@ export async function deploy( KubernetesConstants.DiscoveryAndLoadBalancerResource.SERVICE ]) ) - await checkManifestStability(kubectl, resourceTypes) + await checkManifestStability(kubectl, resourceTypes, timeout) core.endGroup() // print ingresses diff --git a/src/actions/promote.ts b/src/actions/promote.ts index 2c11384c4..a569c9e1a 100644 --- a/src/actions/promote.ts +++ b/src/actions/promote.ts @@ -42,21 +42,26 @@ import {parseRouteStrategy, RouteStrategy} from '../types/routeStrategy' export async function promote( kubectl: Kubectl, manifests: string[], - deploymentStrategy: DeploymentStrategy + deploymentStrategy: DeploymentStrategy, + timeout?: string ) { switch (deploymentStrategy) { case DeploymentStrategy.CANARY: - await promoteCanary(kubectl, manifests) + await promoteCanary(kubectl, manifests, timeout) break case DeploymentStrategy.BLUE_GREEN: - await promoteBlueGreen(kubectl, manifests) + await promoteBlueGreen(kubectl, manifests, timeout) break default: throw Error('Invalid promote deployment strategy') } } -async function promoteCanary(kubectl: Kubectl, manifests: string[]) { +async function promoteCanary( + kubectl: Kubectl, + manifests: string[], + timeout?: string +) { let includeServices = false const manifestFilesForDeployment: string[] = updateManifestFiles(manifests) @@ -85,7 +90,8 @@ async function promoteCanary(kubectl: Kubectl, manifests: string[]) { promoteResult = await SMICanaryDeploymentHelper.deploySMICanary( manifestFilesForDeployment, kubectl, - true + true, + timeout ) core.endGroup() @@ -94,7 +100,8 @@ async function promoteCanary(kubectl: Kubectl, manifests: string[]) { const stableRedirectManifests = await SMICanaryDeploymentHelper.redirectTrafficToStableDeployment( kubectl, - manifests + manifests, + timeout ) filesToAnnotate = promoteResult.manifestFiles.concat( @@ -107,7 +114,8 @@ async function promoteCanary(kubectl: Kubectl, manifests: string[]) { promoteResult = await PodCanaryHelper.deployPodCanary( manifestFilesForDeployment, kubectl, - true + true, + timeout ) filesToAnnotate = promoteResult.manifestFiles core.endGroup() @@ -139,7 +147,11 @@ async function promoteCanary(kubectl: Kubectl, manifests: string[]) { core.endGroup() } -async function promoteBlueGreen(kubectl: Kubectl, manifests: string[]) { +async function promoteBlueGreen( + kubectl: Kubectl, + manifests: string[], + timeout?: string +) { // update container images and pull secrets const inputManifestFiles: string[] = updateManifestFiles(manifests) const manifestObjects: BlueGreenManifests = @@ -173,7 +185,11 @@ async function promoteBlueGreen(kubectl: Kubectl, manifests: string[]) { models.DiscoveryAndLoadBalancerResource.SERVICE ]) ) - await KubernetesManifestUtility.checkManifestStability(kubectl, resources) + await KubernetesManifestUtility.checkManifestStability( + kubectl, + resources, + timeout + ) core.endGroup() core.startGroup( diff --git a/src/actions/reject.ts b/src/actions/reject.ts index 0da91c74d..4c21b0228 100644 --- a/src/actions/reject.ts +++ b/src/actions/reject.ts @@ -19,21 +19,26 @@ import {parseRouteStrategy, RouteStrategy} from '../types/routeStrategy' export async function reject( kubectl: Kubectl, manifests: string[], - deploymentStrategy: DeploymentStrategy + deploymentStrategy: DeploymentStrategy, + timeout?: string ) { switch (deploymentStrategy) { case DeploymentStrategy.CANARY: - await rejectCanary(kubectl, manifests) + await rejectCanary(kubectl, manifests, timeout) break case DeploymentStrategy.BLUE_GREEN: - await rejectBlueGreen(kubectl, manifests) + await rejectBlueGreen(kubectl, manifests, timeout) break default: throw 'Invalid delete deployment strategy' } } -async function rejectCanary(kubectl: Kubectl, manifests: string[]) { +async function rejectCanary( + kubectl: Kubectl, + manifests: string[], + timeout?: string +) { let includeServices = false const trafficSplitMethod = parseTrafficSplitMethod( @@ -44,7 +49,8 @@ async function rejectCanary(kubectl: Kubectl, manifests: string[]) { includeServices = true await SMICanaryDeploymentHelper.redirectTrafficToStableDeployment( kubectl, - manifests + manifests, + timeout ) core.endGroup() } @@ -53,12 +59,17 @@ async function rejectCanary(kubectl: Kubectl, manifests: string[]) { await canaryDeploymentHelper.deleteCanaryDeployment( kubectl, manifests, - includeServices + includeServices, + timeout ) core.endGroup() } -async function rejectBlueGreen(kubectl: Kubectl, manifests: string[]) { +async function rejectBlueGreen( + kubectl: Kubectl, + manifests: string[], + timeout?: string +) { const routeStrategy = parseRouteStrategy( core.getInput('route-method', {required: true}) ) @@ -67,11 +78,11 @@ async function rejectBlueGreen(kubectl: Kubectl, manifests: string[]) { const manifestObjects: BlueGreenManifests = getManifestObjects(manifests) if (routeStrategy == RouteStrategy.INGRESS) { - await rejectBlueGreenIngress(kubectl, manifestObjects) + await rejectBlueGreenIngress(kubectl, manifestObjects, timeout) } else if (routeStrategy == RouteStrategy.SMI) { - await rejectBlueGreenSMI(kubectl, manifestObjects) + await rejectBlueGreenSMI(kubectl, manifestObjects, timeout) } else { - await rejectBlueGreenService(kubectl, manifestObjects) + await rejectBlueGreenService(kubectl, manifestObjects, timeout) } core.endGroup() } diff --git a/src/run.ts b/src/run.ts index 4399345bc..5a9a9a11c 100644 --- a/src/run.ts +++ b/src/run.ts @@ -26,8 +26,9 @@ export async function run() { .map((manifest) => manifest.trim()) // remove surrounding whitespace .filter((manifest) => manifest.length > 0) // remove any blanks - const fullManifestFilePaths = - await getFilesFromDirectoriesAndURLs(manifestFilePaths) + const fullManifestFilePaths = await getFilesFromDirectoriesAndURLs( + manifestFilePaths + ) const kubectlPath = await getKubectlPath() const namespace = core.getInput('namespace') || 'default' const isPrivateCluster = @@ -35,6 +36,7 @@ export async function run() { const resourceGroup = core.getInput('resource-group') || '' const resourceName = core.getInput('name') || '' const skipTlsVerify = core.getBooleanInput('skip-tls-verify') + const timeout = core.getInput('timeout') || '10m' const kubectl = isPrivateCluster ? new PrivateKubectl( @@ -49,15 +51,15 @@ export async function run() { // run action switch (action) { case Action.DEPLOY: { - await deploy(kubectl, fullManifestFilePaths, strategy) + await deploy(kubectl, fullManifestFilePaths, strategy, timeout) break } case Action.PROMOTE: { - await promote(kubectl, fullManifestFilePaths, strategy) + await promote(kubectl, fullManifestFilePaths, strategy, timeout) break } case Action.REJECT: { - await reject(kubectl, fullManifestFilePaths, strategy) + await reject(kubectl, fullManifestFilePaths, strategy, timeout) break } default: { diff --git a/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts b/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts index 6b063ba40..cfacff4c8 100644 --- a/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts +++ b/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts @@ -40,7 +40,8 @@ describe('bluegreenhelper functions', () => { [].concat( testObjects.deploymentEntityList, testObjects.serviceEntityList - ) + ), + '60s' ) expect(value).toHaveLength(2) @@ -52,6 +53,30 @@ describe('bluegreenhelper functions', () => { name: 'nginx-deployment-green', kind: 'Deployment' }) + + expect(bgHelper.deleteObjects).toHaveBeenCalledWith(kubectl, value, '60s') + }) + + test('handles timeout when deleting objects', async () => { + // Mock deleteObjects to prevent actual execution + jest.spyOn(kubectl, 'delete').mockResolvedValue({} as ExecOutput) + + const deleteList = [ + {name: 'nginx-service-green', kind: 'Service'}, + {name: 'nginx-deployment-green', kind: 'Deployment'} + ] + + await bgHelper.deleteObjects(kubectl, deleteList, '120s') + + // Verify kubectl.delete is called with timeout + expect(kubectl.delete).toHaveBeenCalledWith( + ['Service', 'nginx-service-green'], + '120s' + ) + expect(kubectl.delete).toHaveBeenCalledWith( + ['Deployment', 'nginx-deployment-green'], + '120s' + ) }) test('parses objects correctly from one file (getManifestObjects)', () => { diff --git a/src/strategyHelpers/blueGreen/blueGreenHelper.ts b/src/strategyHelpers/blueGreen/blueGreenHelper.ts index 42704740c..399fe7cd2 100644 --- a/src/strategyHelpers/blueGreen/blueGreenHelper.ts +++ b/src/strategyHelpers/blueGreen/blueGreenHelper.ts @@ -32,7 +32,8 @@ export const STABLE_SUFFIX = '-stable' export async function deleteGreenObjects( kubectl: Kubectl, - toDelete: K8sObject[] + toDelete: K8sObject[], + timeout?: string ): Promise { // const resourcesToDelete: K8sDeleteObject[] = [] const resourcesToDelete: K8sDeleteObject[] = toDelete.map((obj) => { @@ -45,18 +46,22 @@ export async function deleteGreenObjects( core.debug(`deleting green objects: ${JSON.stringify(resourcesToDelete)}`) - await deleteObjects(kubectl, resourcesToDelete) + await deleteObjects(kubectl, resourcesToDelete, timeout) return resourcesToDelete } export async function deleteObjects( kubectl: Kubectl, - deleteList: K8sDeleteObject[] + deleteList: K8sDeleteObject[], + timeout?: string ) { // delete services and deployments for (const delObject of deleteList) { try { - const result = await kubectl.delete([delObject.kind, delObject.name]) + const result = await kubectl.delete( + [delObject.kind, delObject.name], + timeout + ) checkForErrors([result]) } catch (ex) { core.debug(`failed to delete object ${delObject.name}: ${ex}`) diff --git a/src/strategyHelpers/blueGreen/reject.test.ts b/src/strategyHelpers/blueGreen/reject.test.ts index 7a6c1ae1b..3a831304b 100644 --- a/src/strategyHelpers/blueGreen/reject.test.ts +++ b/src/strategyHelpers/blueGreen/reject.test.ts @@ -8,6 +8,8 @@ import { rejectBlueGreenService, rejectBlueGreenSMI } from './reject' +import * as bgHelper from './blueGreenHelper' +import * as routeHelper from './route' const ingressFilepath = ['test/unit/manifests/test-ingress-new.yml'] const kubectl = new Kubectl('') @@ -43,6 +45,55 @@ describe('reject tests', () => { expect(bgDeployment.objects[0].metadata.name).toBe('nginx-ingress') }) + test('reject blue/green ingress with timeout', async () => { + // Mock routeBlueGreenIngressUnchanged and deleteGreenObjects + jest + .spyOn(routeHelper, 'routeBlueGreenIngressUnchanged') + .mockResolvedValue({ + deployResult: {}, + objects: [{metadata: {name: 'nginx-ingress'}}] + }) + + jest.spyOn(bgHelper, 'deleteGreenObjects').mockResolvedValue([ + {name: 'nginx-service-green', kind: 'Service'}, + {name: 'nginx-deployment-green', kind: 'Deployment'} + ]) + + const value = await rejectBlueGreenIngress(kubectl, testObjects, '120s') + + const bgDeployment = value.routeResult + const deleteResult = value.deleteResult + + expect(deleteResult).toHaveLength(2) + for (const obj of deleteResult) { + if (obj.kind === 'Service') { + expect(obj.name).toBe('nginx-service-green') + } + if (obj.kind === 'Deployment') { + expect(obj.name).toBe('nginx-deployment-green') + } + } + + expect(bgDeployment.objects).toHaveLength(1) + expect(bgDeployment.objects[0].metadata.name).toBe('nginx-ingress') + + // Verify deleteGreenObjects is called with timeout + expect(bgHelper.deleteGreenObjects).toHaveBeenCalledWith( + kubectl, + [].concat( + testObjects.deploymentEntityList, + testObjects.serviceEntityList + ), + '120s' + ) + expect(routeHelper.routeBlueGreenIngressUnchanged).toHaveBeenCalledWith( + kubectl, + testObjects.serviceNameMap, + testObjects.ingressEntityList, + '120s' + ) + }) + test('reject blue/green service', async () => { const value = await rejectBlueGreenService(kubectl, testObjects) @@ -56,6 +107,39 @@ describe('reject tests', () => { expect(bgDeployment.objects[0].metadata.name).toBe('nginx-service') }) + test('reject blue/green service with timeout', async () => { + // Mock routeBlueGreenService and deleteGreenObjects + jest.spyOn(routeHelper, 'routeBlueGreenService').mockResolvedValue({ + deployResult: {}, + objects: [{metadata: {name: 'nginx-service'}}] + }) + + jest + .spyOn(bgHelper, 'deleteGreenObjects') + .mockResolvedValue([ + {name: 'nginx-deployment-green', kind: 'Deployment'} + ]) + + const value = await rejectBlueGreenService(kubectl, testObjects, '120s') + + const bgDeployment = value.routeResult + const deleteResult = value.deleteResult + + // Verify deleteGreenObjects is called with timeout + expect(bgHelper.deleteGreenObjects).toHaveBeenCalledWith( + kubectl, + testObjects.deploymentEntityList, + '120s' + ) + + // Assertions for routeResult and deleteResult + expect(deleteResult).toHaveLength(1) + expect(deleteResult[0].name).toBe('nginx-deployment-green') + + expect(bgDeployment.objects).toHaveLength(1) + expect(bgDeployment.objects[0].metadata.name).toBe('nginx-service') + }) + test('reject blue/green SMI', async () => { jest .spyOn(TSutils, 'getTrafficSplitAPIVersion') diff --git a/src/strategyHelpers/blueGreen/reject.ts b/src/strategyHelpers/blueGreen/reject.ts index c0be91e28..b07020098 100644 --- a/src/strategyHelpers/blueGreen/reject.ts +++ b/src/strategyHelpers/blueGreen/reject.ts @@ -12,14 +12,16 @@ import {routeBlueGreenIngressUnchanged, routeBlueGreenService} from './route' export async function rejectBlueGreenIngress( kubectl: Kubectl, - manifestObjects: BlueGreenManifests + manifestObjects: BlueGreenManifests, + timeout?: string ): Promise { // get all kubernetes objects defined in manifest files // route ingress to stables services const routeResult = await routeBlueGreenIngressUnchanged( kubectl, manifestObjects.serviceNameMap, - manifestObjects.ingressEntityList + manifestObjects.ingressEntityList, + timeout ) // delete green services and deployments @@ -28,7 +30,8 @@ export async function rejectBlueGreenIngress( [].concat( manifestObjects.deploymentEntityList, manifestObjects.serviceEntityList - ) + ), + timeout ) return {routeResult, deleteResult} @@ -36,19 +39,22 @@ export async function rejectBlueGreenIngress( export async function rejectBlueGreenService( kubectl: Kubectl, - manifestObjects: BlueGreenManifests + manifestObjects: BlueGreenManifests, + timeout?: string ): Promise { // route to stable objects const routeResult = await routeBlueGreenService( kubectl, NONE_LABEL_VALUE, - manifestObjects.serviceEntityList + manifestObjects.serviceEntityList, + timeout ) // delete new deployments with green suffix const deleteResult = await deleteGreenObjects( kubectl, - manifestObjects.deploymentEntityList + manifestObjects.deploymentEntityList, + timeout ) return {routeResult, deleteResult} @@ -56,25 +62,29 @@ export async function rejectBlueGreenService( export async function rejectBlueGreenSMI( kubectl: Kubectl, - manifestObjects: BlueGreenManifests + manifestObjects: BlueGreenManifests, + timeout?: string ): Promise { // route trafficsplit to stable deployments const routeResult = await routeBlueGreenSMI( kubectl, NONE_LABEL_VALUE, - manifestObjects.serviceEntityList + manifestObjects.serviceEntityList, + timeout ) // delete rejected new bluegreen deployments const deletedObjects = await deleteGreenObjects( kubectl, - manifestObjects.deploymentEntityList + manifestObjects.deploymentEntityList, + timeout ) // delete trafficsplit and extra services const cleanupResult = await cleanupSMI( kubectl, - manifestObjects.serviceEntityList + manifestObjects.serviceEntityList, + timeout ) return {routeResult, deleteResult: [].concat(deletedObjects, cleanupResult)} diff --git a/src/strategyHelpers/blueGreen/route.ts b/src/strategyHelpers/blueGreen/route.ts index f0c385517..3b1cf76e5 100644 --- a/src/strategyHelpers/blueGreen/route.ts +++ b/src/strategyHelpers/blueGreen/route.ts @@ -92,26 +92,28 @@ export async function routeBlueGreenIngress( export async function routeBlueGreenIngressUnchanged( kubectl: Kubectl, serviceNameMap: Map, - ingressEntityList: any[] + ingressEntityList: any[], + timeout?: string ): Promise { const objects = ingressEntityList.filter((ingress) => isIngressRouted(ingress, serviceNameMap) ) - const deployResult = await deployObjects(kubectl, objects) + const deployResult = await deployObjects(kubectl, objects, timeout) return {deployResult, objects} } export async function routeBlueGreenService( kubectl: Kubectl, nextLabel: string, - serviceEntityList: any[] + serviceEntityList: any[], + timeout?: string ): Promise { const objects = serviceEntityList.map((serviceObject) => getUpdatedBlueGreenService(serviceObject, nextLabel) ) - const deployResult = await deployObjects(kubectl, objects) + const deployResult = await deployObjects(kubectl, objects, timeout) return {deployResult, objects} } @@ -119,7 +121,8 @@ export async function routeBlueGreenService( export async function routeBlueGreenSMI( kubectl: Kubectl, nextLabel: string, - serviceEntityList: any[] + serviceEntityList: any[], + timeout?: string ): Promise { // let tsObjects: TrafficSplitObject[] = [] @@ -128,14 +131,15 @@ export async function routeBlueGreenSMI( const tsObject: TrafficSplitObject = await createTrafficSplitObject( kubectl, serviceObject.metadata.name, - nextLabel + nextLabel, + timeout ) return tsObject }) ) - const deployResult = await deployObjects(kubectl, tsObjects) + const deployResult = await deployObjects(kubectl, tsObjects, timeout) return {deployResult, objects: tsObjects} } diff --git a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts index 2d71e2f9e..dddea48a8 100644 --- a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts +++ b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts @@ -191,10 +191,32 @@ describe('SMI Helper tests', () => { expect(valResult).toBe(false) }) - test('cleanupSMI test', async () => { + test('cleanupSMI test without timeout', async () => { const deleteObjects = await cleanupSMI(kc, testObjects.serviceEntityList) expect(deleteObjects).toHaveLength(1) expect(deleteObjects[0].name).toBe('nginx-service-green') expect(deleteObjects[0].kind).toBe('Service') }) + + test('cleanupSMI test with timeout', async () => { + jest.spyOn(bgHelper, 'deleteObjects').mockResolvedValue() + + // Call cleanupSMI with a timeout + const deleteObjects = await cleanupSMI( + kc, + testObjects.serviceEntityList, + '120s' + ) + + // Assertions for returned delete list + expect(deleteObjects).toHaveLength(1) + expect(deleteObjects[0].name).toBe('nginx-service-green') + expect(deleteObjects[0].kind).toBe('Service') + + expect(bgHelper.deleteObjects).toHaveBeenCalledWith( + kc, + deleteObjects, + '120s' + ) + }) }) diff --git a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts index efdfe4ea0..9060b9105 100644 --- a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts +++ b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts @@ -77,8 +77,9 @@ export async function createTrafficSplitObject( ): Promise { // cache traffic split api version if (!trafficSplitAPIVersion) - trafficSplitAPIVersion = - await kubectlUtils.getTrafficSplitAPIVersion(kubectl) + trafficSplitAPIVersion = await kubectlUtils.getTrafficSplitAPIVersion( + kubectl + ) // retrieve annotations for TS object const annotations = inputAnnotations @@ -173,7 +174,8 @@ export async function validateTrafficSplitsState( export async function cleanupSMI( kubectl: Kubectl, - serviceEntityList: any[] + serviceEntityList: any[], + timeout?: string ): Promise { const deleteList: K8sDeleteObject[] = [] @@ -189,7 +191,7 @@ export async function cleanupSMI( }) // delete all objects - await deleteObjects(kubectl, deleteList) + await deleteObjects(kubectl, deleteList, timeout) return deleteList } diff --git a/src/strategyHelpers/canary/canaryHelper.ts b/src/strategyHelpers/canary/canaryHelper.ts index 20c7eebfa..2b748b3a1 100644 --- a/src/strategyHelpers/canary/canaryHelper.ts +++ b/src/strategyHelpers/canary/canaryHelper.ts @@ -28,7 +28,8 @@ export const STABLE_LABEL_VALUE = 'stable' export async function deleteCanaryDeployment( kubectl: Kubectl, manifestFilePaths: string[], - includeServices: boolean + includeServices: boolean, + timeout?: string ): Promise { if (manifestFilePaths == null || manifestFilePaths.length == 0) { throw new Error('Manifest files for deleting canary deployment not found') @@ -37,7 +38,8 @@ export async function deleteCanaryDeployment( const deletedFiles = await cleanUpCanary( kubectl, manifestFilePaths, - includeServices + includeServices, + timeout ) return deletedFiles } @@ -83,11 +85,12 @@ export function getNewCanaryResource( export async function fetchResource( kubectl: Kubectl, kind: string, - name: string + name: string, + timeout?: string ) { let result: ExecOutput try { - result = await kubectl.getResource(kind, name) + result = await kubectl.getResource(kind, name, false, undefined, timeout) } catch (e) { core.debug(`detected error while fetching resources: ${e}`) } @@ -193,7 +196,8 @@ function addCanaryLabelsAndAnnotations(inputObject: any, type: string) { async function cleanUpCanary( kubectl: Kubectl, files: string[], - includeServices: boolean + includeServices: boolean, + timeout?: string ): Promise { const deleteObject = async function ( kind: string, @@ -201,7 +205,7 @@ async function cleanUpCanary( namespace: string | undefined ) { try { - const result = await kubectl.delete([kind, name], namespace) + const result = await kubectl.delete([kind, name], namespace, timeout) checkForErrors([result]) } catch (ex) { // Ignore failures of delete if it doesn't exist diff --git a/src/strategyHelpers/canary/podCanaryHelper.ts b/src/strategyHelpers/canary/podCanaryHelper.ts index da769aeea..14efa410e 100644 --- a/src/strategyHelpers/canary/podCanaryHelper.ts +++ b/src/strategyHelpers/canary/podCanaryHelper.ts @@ -13,7 +13,8 @@ import {K8sObject} from '../../types/k8sObject' export async function deployPodCanary( filePaths: string[], kubectl: Kubectl, - onlyDeployStable: boolean = false + onlyDeployStable: boolean = false, + timeout?: string ): Promise { const newObjectsList = [] const percentage = parseInt(core.getInput('percentage', {required: true})) @@ -61,7 +62,8 @@ export async function deployPodCanary( await canaryDeploymentHelper.fetchResource( kubectl, kind, - name + name, + timeout ) if (stableObject) { core.debug( diff --git a/src/strategyHelpers/canary/smiCanaryHelper.ts b/src/strategyHelpers/canary/smiCanaryHelper.ts index ee5918bd7..cd46218a5 100644 --- a/src/strategyHelpers/canary/smiCanaryHelper.ts +++ b/src/strategyHelpers/canary/smiCanaryHelper.ts @@ -19,7 +19,8 @@ const TRAFFIC_SPLIT_OBJECT = 'TrafficSplit' export async function deploySMICanary( filePaths: string[], kubectl: Kubectl, - onlyDeployStable: boolean = false + onlyDeployStable: boolean = false, + timeout?: string ): Promise { const canaryReplicasInput = core.getInput('baseline-and-canary-replicas') let canaryReplicaCount @@ -107,14 +108,19 @@ export async function deploySMICanary( const newFilePaths = fileHelper.writeObjectsToFile(newObjectsList) const forceDeployment = core.getInput('force').toLowerCase() === 'true' const result = await kubectl.apply(newFilePaths, forceDeployment) - const svcDeploymentFiles = await createCanaryService(kubectl, filePaths) + const svcDeploymentFiles = await createCanaryService( + kubectl, + filePaths, + timeout + ) newFilePaths.push(...svcDeploymentFiles) return {execResult: result, manifestFiles: newFilePaths} } async function createCanaryService( kubectl: Kubectl, - filePaths: string[] + filePaths: string[], + timeout?: string ): Promise { const newObjectsList = [] const trafficObjectsList: string[] = [] @@ -156,7 +162,8 @@ async function createCanaryService( name, 0, 0, - 1000 + 1000, + timeout ) trafficObjectsList.push(trafficObject) @@ -225,16 +232,18 @@ export async function redirectTrafficToCanaryDeployment( export async function redirectTrafficToStableDeployment( kubectl: Kubectl, - manifestFilePaths: string[] + manifestFilePaths: string[], + timeout?: string ): Promise { - return await adjustTraffic(kubectl, manifestFilePaths, 1000, 0) + return await adjustTraffic(kubectl, manifestFilePaths, 1000, 0, timeout) } async function adjustTraffic( kubectl: Kubectl, manifestFilePaths: string[], stableWeight: number, - canaryWeight: number + canaryWeight: number, + timeout?: string ) { if (!manifestFilePaths || manifestFilePaths?.length == 0) { return @@ -259,7 +268,8 @@ async function adjustTraffic( name, stableWeight, 0, - canaryWeight + canaryWeight, + timeout ) ) } @@ -314,14 +324,16 @@ async function createTrafficSplitManifestFile( serviceName: string, stableWeight: number, baselineWeight: number, - canaryWeight: number + canaryWeight: number, + timeout?: string ): Promise { const smiObjectString = await getTrafficSplitObject( kubectl, serviceName, stableWeight, baselineWeight, - canaryWeight + canaryWeight, + timeout ) const manifestFile = fileHelper.writeManifestToFile( smiObjectString, @@ -343,12 +355,14 @@ async function getTrafficSplitObject( name: string, stableWeight: number, baselineWeight: number, - canaryWeight: number + canaryWeight: number, + timeout?: string ): Promise { // cached version if (!trafficSplitAPIVersion) { - trafficSplitAPIVersion = - await kubectlUtils.getTrafficSplitAPIVersion(kubectl) + trafficSplitAPIVersion = await kubectlUtils.getTrafficSplitAPIVersion( + kubectl + ) } return JSON.stringify({ diff --git a/src/strategyHelpers/deploymentHelper.ts b/src/strategyHelpers/deploymentHelper.ts index 24257701e..7b21f8b04 100644 --- a/src/strategyHelpers/deploymentHelper.ts +++ b/src/strategyHelpers/deploymentHelper.ts @@ -138,9 +138,14 @@ function appendStableVersionLabelToResource(files: string[]): string[] { export async function checkManifestStability( kubectl: Kubectl, - resources: Resource[] + resources: Resource[], + timeout?: string ): Promise { - await KubernetesManifestUtility.checkManifestStability(kubectl, resources) + await KubernetesManifestUtility.checkManifestStability( + kubectl, + resources, + timeout + ) } export async function annotateAndLabelResources( diff --git a/src/types/kubectl.test.ts b/src/types/kubectl.test.ts index 91cbf6479..d5bae3d2a 100644 --- a/src/types/kubectl.test.ts +++ b/src/types/kubectl.test.ts @@ -41,18 +41,18 @@ const otherNamespace = 'otherns' describe('Kubectl class', () => { describe('with a success exec return in testNamespace', () => { const kubectl = new Kubectl(kubectlPath, testNamespace) - const execReturn = {exitCode: 0, stdout: 'Output', stderr: ''} + const mockExecReturn = {exitCode: 0, stdout: 'Output', stderr: ''} beforeEach(() => { jest.spyOn(exec, 'getExecOutput').mockImplementation(async () => { - return execReturn + return mockExecReturn }) }) it('applies a configuration with a single config path', async () => { const configPaths = 'configPaths' const result = await kubectl.apply(configPaths) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, ['apply', '-f', configPaths, '--namespace', testNamespace], @@ -63,7 +63,7 @@ describe('Kubectl class', () => { it('applies a configuration with multiple config paths', async () => { const configPaths = ['configPath1', 'configPath2', 'configPath3'] const result = await kubectl.apply(configPaths) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -80,7 +80,7 @@ describe('Kubectl class', () => { it('applies a configuration with force when specified', async () => { const configPaths = ['configPath1', 'configPath2', 'configPath3'] const result = await kubectl.apply(configPaths, true) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -99,7 +99,7 @@ describe('Kubectl class', () => { const resourceType = 'type' const resourceName = 'name' const result = await kubectl.describe(resourceType, resourceName) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -137,7 +137,7 @@ describe('Kubectl class', () => { const resourceType = 'type' const resourceName = 'name' const result = await kubectl.describe(resourceType, resourceName, true) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -180,7 +180,7 @@ describe('Kubectl class', () => { resourceName, annotation ) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -221,7 +221,7 @@ describe('Kubectl class', () => { const file = 'file' const annotation = 'annotation' const result = await kubectl.annotateFiles(file, annotation) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -257,7 +257,7 @@ describe('Kubectl class', () => { const files = ['file1', 'file2', 'file3'] const annotation = 'annotation' const result = await kubectl.annotateFiles(files, annotation) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -293,7 +293,7 @@ describe('Kubectl class', () => { const file = 'file' const labels = ['label1', 'label2'] const result = await kubectl.labelFiles(file, labels) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -328,7 +328,7 @@ describe('Kubectl class', () => { const files = ['file1', 'file2', 'file3'] const labels = ['label1', 'label2'] const result = await kubectl.labelFiles(files, labels) - expect(result).toBe(execReturn) + expect(result).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -360,7 +360,7 @@ describe('Kubectl class', () => { }) it('gets all pods', async () => { - expect(await kubectl.getAllPods()).toBe(execReturn) + expect(await kubectl.getAllPods()).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, ['get', 'pods', '-o', 'json', '--namespace', testNamespace], @@ -373,7 +373,7 @@ describe('Kubectl class', () => { const name = 'name' const timeout = '60s' expect(await kubectl.checkRolloutStatus(resourceType, name)).toBe( - execReturn + mockExecReturn ) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, @@ -445,7 +445,9 @@ describe('Kubectl class', () => { it('gets resource', async () => { const resourceType = 'type' const name = 'name' - expect(await kubectl.getResource(resourceType, name)).toBe(execReturn) + expect(await kubectl.getResource(resourceType, name)).toBe( + mockExecReturn + ) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [ @@ -479,7 +481,7 @@ describe('Kubectl class', () => { it('executes a command', async () => { // no args const command = 'command' - expect(await kubectl.executeCommand(command)).toBe(execReturn) + expect(await kubectl.executeCommand(command)).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [command, '--namespace', testNamespace], @@ -488,17 +490,37 @@ describe('Kubectl class', () => { // with args const args = 'args' - expect(await kubectl.executeCommand(command, args)).toBe(execReturn) + expect(await kubectl.executeCommand(command, args)).toBe( + mockExecReturn + ) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, [command, args, '--namespace', testNamespace], {silent: false} ) + + // with args and timeout + const timeout = '60s' + console.log('Testing executeCommand with timeout:', timeout) + expect(await kubectl.executeCommand(command, args, timeout)).toBe( + mockExecReturn + ) + expect(exec.getExecOutput).toHaveBeenCalledWith( + kubectlPath, + [ + 'command', + 'args', + '--namespace', + testNamespace, + `--timeout=${timeout}` + ], + {silent: false} + ) }) it('deletes with single argument', async () => { const arg = 'argument' - expect(await kubectl.delete(arg)).toBe(execReturn) + expect(await kubectl.delete(arg)).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, ['delete', arg, '--namespace', testNamespace], @@ -516,7 +538,7 @@ describe('Kubectl class', () => { it('deletes with multiple arguments', async () => { const args = ['argument1', 'argument2', 'argument3'] - expect(await kubectl.delete(args)).toBe(execReturn) + expect(await kubectl.delete(args)).toBe(mockExecReturn) expect(exec.getExecOutput).toBeCalledWith( kubectlPath, ['delete', ...args, '--namespace', testNamespace], diff --git a/src/types/kubectl.ts b/src/types/kubectl.ts index dcc080645..8cd9d88bd 100644 --- a/src/types/kubectl.ts +++ b/src/types/kubectl.ts @@ -173,7 +173,8 @@ export class Kubectl { resourceType: string, name: string, silentFailure: boolean = false, - namespace?: string + namespace?: string, + timeout?: string ): Promise { core.debug( 'fetching resource of type ' + resourceType + ' and name ' + name @@ -182,24 +183,54 @@ export class Kubectl { ['get', `${resourceType}/${name}`, '-o', 'json'].concat( this.getFlags(namespace) ), - silentFailure + silentFailure, + timeout ) } - public executeCommand(command: string, args?: string) { + public executeCommand(command: string, args?: string, timeout?: string) { if (!command) throw new Error('Command must be defined') const a = args ? [args] : [] - return this.execute([command, ...a.concat(this.getFlags())]) + return this.execute( + [command, ...a.concat(this.getFlags())], + false, + timeout + ) } - public delete(args: string | string[], namespace?: string) { + public delete( + args: string | string[], + namespace?: string, + timeout?: string + ) { if (typeof args === 'string') - return this.execute(['delete', args].concat(this.getFlags(namespace))) - return this.execute(['delete', ...args.concat(this.getFlags(namespace))]) + return this.execute( + ['delete', args].concat(this.getFlags(namespace)), + false, + timeout + ) + return this.execute( + ['delete', ...args.concat(this.getFlags(namespace))], + false, + timeout + ) } - protected async execute(args: string[], silent: boolean = false) { - core.debug(`Kubectl run with command: ${this.kubectlPath} ${args}`) + protected async execute( + args: string[], + silent: boolean = false, + timeout?: string + ) { + // core.debug(`Kubectl run with command: ${this.kubectlPath} ${args}`) + core.debug( + `Kubectl run with command: ${this.kubectlPath} ${args.join(' ')}` + ) + + if (timeout) { + args.push(`--timeout=${timeout}`) + } + + console.log('Final command args:', args) return await getExecOutput(this.kubectlPath, args, { silent From bdf6a4740f199f11862afe7026fb6a31a172ed96 Mon Sep 17 00:00:00 2001 From: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:17:23 -0800 Subject: [PATCH 3/5] updating the unit tests --- .../blueGreen/blueGreenHelper.test.ts | 14 +++++------- src/strategyHelpers/blueGreen/reject.test.ts | 22 +++++++++++++------ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts b/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts index cfacff4c8..bc218a53a 100644 --- a/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts +++ b/src/strategyHelpers/blueGreen/blueGreenHelper.test.ts @@ -53,29 +53,27 @@ describe('bluegreenhelper functions', () => { name: 'nginx-deployment-green', kind: 'Deployment' }) - - expect(bgHelper.deleteObjects).toHaveBeenCalledWith(kubectl, value, '60s') }) test('handles timeout when deleting objects', async () => { - // Mock deleteObjects to prevent actual execution - jest.spyOn(kubectl, 'delete').mockResolvedValue({} as ExecOutput) - const deleteList = [ {name: 'nginx-service-green', kind: 'Service'}, {name: 'nginx-deployment-green', kind: 'Deployment'} ] - await bgHelper.deleteObjects(kubectl, deleteList, '120s') + // Mock deleteObjects to prevent actual execution + jest.spyOn(kubectl, 'delete').mockResolvedValue({} as ExecOutput) + + await bgHelper.deleteObjects(kubectl, deleteList, '60s') // Verify kubectl.delete is called with timeout expect(kubectl.delete).toHaveBeenCalledWith( ['Service', 'nginx-service-green'], - '120s' + '60s' ) expect(kubectl.delete).toHaveBeenCalledWith( ['Deployment', 'nginx-deployment-green'], - '120s' + '60s' ) }) diff --git a/src/strategyHelpers/blueGreen/reject.test.ts b/src/strategyHelpers/blueGreen/reject.test.ts index 3a831304b..1fce7be02 100644 --- a/src/strategyHelpers/blueGreen/reject.test.ts +++ b/src/strategyHelpers/blueGreen/reject.test.ts @@ -95,16 +95,24 @@ describe('reject tests', () => { }) test('reject blue/green service', async () => { - const value = await rejectBlueGreenService(kubectl, testObjects) + jest.spyOn(bgHelper, 'deleteGreenObjects').mockResolvedValue([ + {name: 'nginx-service-green', kind: 'Service'}, + {name: 'nginx-deployment-green', kind: 'Deployment'} + ]) - const bgDeployment = value.routeResult - const deleteResult = value.deleteResult + const value = await rejectBlueGreenService(kubectl, testObjects, '60s') - expect(deleteResult).toHaveLength(1) - expect(deleteResult[0].name).toBe('nginx-deployment-green') + const deleteResult = value.deleteResult - expect(bgDeployment.objects).toHaveLength(1) - expect(bgDeployment.objects[0].metadata.name).toBe('nginx-service') + expect(deleteResult).toHaveLength(2) + expect(deleteResult).toContainEqual({ + name: 'nginx-service-green', + kind: 'Service' + }) + expect(deleteResult).toContainEqual({ + name: 'nginx-deployment-green', + kind: 'Deployment' + }) }) test('reject blue/green service with timeout', async () => { From f7107d42ddf88f74a2ee69cfdac302cd8969a978 Mon Sep 17 00:00:00 2001 From: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:42:30 -0800 Subject: [PATCH 4/5] updated the functions to address build errors --- .../blueGreen/blueGreenHelper.ts | 5 +- .../blueGreen/smiBlueGreenHelper.test.ts | 77 ++++++++++++++----- .../blueGreen/smiBlueGreenHelper.ts | 10 ++- src/types/kubectl.test.ts | 39 ++++++++++ src/types/kubectl.ts | 7 +- 5 files changed, 114 insertions(+), 24 deletions(-) diff --git a/src/strategyHelpers/blueGreen/blueGreenHelper.ts b/src/strategyHelpers/blueGreen/blueGreenHelper.ts index 399fe7cd2..23801aa31 100644 --- a/src/strategyHelpers/blueGreen/blueGreenHelper.ts +++ b/src/strategyHelpers/blueGreen/blueGreenHelper.ts @@ -272,10 +272,11 @@ export async function fetchResource( export async function deployObjects( kubectl: Kubectl, - objectsList: any[] + objectsList: any[], + timeout?: string ): Promise { const manifestFiles = fileHelper.writeObjectsToFile(objectsList) - const execResult = await kubectl.apply(manifestFiles) + const execResult = await kubectl.apply(manifestFiles, false, timeout) return {execResult, manifestFiles} } diff --git a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts index dddea48a8..04381b043 100644 --- a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts +++ b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.test.ts @@ -114,30 +114,54 @@ describe('SMI Helper tests', () => { testObjects.serviceEntityList[0].metadata.name, NONE_LABEL_VALUE ) - expect(noneTsObject.metadata.name).toBe('nginx-service-trafficsplit') - for (let be of noneTsObject.spec.backends) { - if (be.service === 'nginx-service-stable') { - expect(be.weight).toBe(MAX_VAL) - } - if (be.service === 'nginx-service-green') { - expect(be.weight).toBe(MIN_VAL) - } - } + validateTrafficSplitObject( + noneTsObject, + 'nginx-service-trafficsplit', + MAX_VAL, + MIN_VAL + ) const greenTsObject: TrafficSplitObject = await createTrafficSplitObject( kc, testObjects.serviceEntityList[0].metadata.name, GREEN_LABEL_VALUE ) - expect(greenTsObject.metadata.name).toBe('nginx-service-trafficsplit') - for (const be of greenTsObject.spec.backends) { - if (be.service === 'nginx-service-stable') { - expect(be.weight).toBe(MIN_VAL) - } - if (be.service === 'nginx-service-green') { - expect(be.weight).toBe(MAX_VAL) - } - } + validateTrafficSplitObject( + greenTsObject, + 'nginx-service-trafficsplit', + MIN_VAL, + MAX_VAL + ) + }) + + test('createTrafficSplitObject tests with timeout', async () => { + const timeout = '60s' + + const noneTsObject: TrafficSplitObject = await createTrafficSplitObject( + kc, + testObjects.serviceEntityList[0].metadata.name, + NONE_LABEL_VALUE, + timeout + ) + validateTrafficSplitObject( + noneTsObject, + 'nginx-service-trafficsplit', + MAX_VAL, + MIN_VAL + ) + + const greenTsObject: TrafficSplitObject = await createTrafficSplitObject( + kc, + testObjects.serviceEntityList[0].metadata.name, + GREEN_LABEL_VALUE, + timeout + ) + validateTrafficSplitObject( + greenTsObject, + 'nginx-service-trafficsplit', + MIN_VAL, + MAX_VAL + ) }) test('getSMIServiceResource test', () => { @@ -220,3 +244,20 @@ describe('SMI Helper tests', () => { ) }) }) + +function validateTrafficSplitObject( + tsObject: TrafficSplitObject, + expectedName: string, + expectedStableWeight: number, + expectedGreenWeight: number +) { + expect(tsObject.metadata.name).toBe(expectedName) + for (const be of tsObject.spec.backends) { + if (be.service === 'nginx-service-stable') { + expect(be.weight).toBe(expectedStableWeight) + } + if (be.service === 'nginx-service-green') { + expect(be.weight).toBe(expectedGreenWeight) + } + } +} diff --git a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts index 9060b9105..28fc469c3 100644 --- a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts +++ b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts @@ -73,7 +73,8 @@ let trafficSplitAPIVersion = '' export async function createTrafficSplitObject( kubectl: Kubectl, name: string, - nextLabel: string + nextLabel: string, + timeout?: string ): Promise { // cache traffic split api version if (!trafficSplitAPIVersion) @@ -113,6 +114,13 @@ export async function createTrafficSplitObject( } } + const deleteList: K8sDeleteObject[] = [ + { + name: trafficSplitObject.metadata.name, + kind: trafficSplitObject.kind + } + ] + await deleteObjects(kubectl, deleteList, timeout) return trafficSplitObject } diff --git a/src/types/kubectl.test.ts b/src/types/kubectl.test.ts index d5bae3d2a..da0aa5ebd 100644 --- a/src/types/kubectl.test.ts +++ b/src/types/kubectl.test.ts @@ -95,6 +95,45 @@ describe('Kubectl class', () => { ) }) + it('applies a configuration with timeout when specified', async () => { + const configPaths = ['configPath1', 'configPath2', 'configPath3'] + const timeout = '120s' + const result = await kubectl.apply(configPaths, false, timeout) + expect(result).toBe(mockExecReturn) + expect(exec.getExecOutput).toBeCalledWith( + kubectlPath, + [ + 'apply', + '-f', + configPaths[0] + ',' + configPaths[1] + ',' + configPaths[2], + `--timeout=${timeout}`, + '--namespace', + testNamespace + ], + {silent: false} + ) + }) + + it('applies a configuration with force and timeout when specified', async () => { + const configPaths = ['configPath1', 'configPath2', 'configPath3'] + const timeout = '120s' + const result = await kubectl.apply(configPaths, true, timeout) + expect(result).toBe(mockExecReturn) + expect(exec.getExecOutput).toBeCalledWith( + kubectlPath, + [ + 'apply', + '-f', + configPaths[0] + ',' + configPaths[1] + ',' + configPaths[2], + '--force', + `--timeout=${timeout}`, + '--namespace', + testNamespace + ], + {silent: false} + ) + }) + it('describes a resource', async () => { const resourceType = 'type' const resourceName = 'name' diff --git a/src/types/kubectl.ts b/src/types/kubectl.ts index 8cd9d88bd..80002b764 100644 --- a/src/types/kubectl.ts +++ b/src/types/kubectl.ts @@ -34,7 +34,8 @@ export class Kubectl { public async apply( configurationPaths: string | string[], - force: boolean = false + force: boolean = false, + timeout?: string ): Promise { try { if (!configurationPaths || configurationPaths?.length === 0) @@ -46,10 +47,12 @@ export class Kubectl { createInlineArray(configurationPaths) ] if (force) applyArgs.push('--force') + if (timeout) applyArgs.push(`--timeout=${timeout}`) return await this.execute(applyArgs.concat(this.getFlags())) } catch (err) { core.debug('Kubectl apply failed:' + err) + throw err } } @@ -230,8 +233,6 @@ export class Kubectl { args.push(`--timeout=${timeout}`) } - console.log('Final command args:', args) - return await getExecOutput(this.kubectlPath, args, { silent }) From 1f0e773ff5bf9dcca98dfdc3771707f186d6e5e0 Mon Sep 17 00:00:00 2001 From: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:54:14 -0800 Subject: [PATCH 5/5] prettier code --- src/run.ts | 5 ++--- src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts | 5 ++--- src/strategyHelpers/canary/smiCanaryHelper.ts | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/run.ts b/src/run.ts index 5a9a9a11c..af96a25a6 100644 --- a/src/run.ts +++ b/src/run.ts @@ -26,9 +26,8 @@ export async function run() { .map((manifest) => manifest.trim()) // remove surrounding whitespace .filter((manifest) => manifest.length > 0) // remove any blanks - const fullManifestFilePaths = await getFilesFromDirectoriesAndURLs( - manifestFilePaths - ) + const fullManifestFilePaths = + await getFilesFromDirectoriesAndURLs(manifestFilePaths) const kubectlPath = await getKubectlPath() const namespace = core.getInput('namespace') || 'default' const isPrivateCluster = diff --git a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts index 28fc469c3..d926462e2 100644 --- a/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts +++ b/src/strategyHelpers/blueGreen/smiBlueGreenHelper.ts @@ -78,9 +78,8 @@ export async function createTrafficSplitObject( ): Promise { // cache traffic split api version if (!trafficSplitAPIVersion) - trafficSplitAPIVersion = await kubectlUtils.getTrafficSplitAPIVersion( - kubectl - ) + trafficSplitAPIVersion = + await kubectlUtils.getTrafficSplitAPIVersion(kubectl) // retrieve annotations for TS object const annotations = inputAnnotations diff --git a/src/strategyHelpers/canary/smiCanaryHelper.ts b/src/strategyHelpers/canary/smiCanaryHelper.ts index cd46218a5..89cacab55 100644 --- a/src/strategyHelpers/canary/smiCanaryHelper.ts +++ b/src/strategyHelpers/canary/smiCanaryHelper.ts @@ -360,9 +360,8 @@ async function getTrafficSplitObject( ): Promise { // cached version if (!trafficSplitAPIVersion) { - trafficSplitAPIVersion = await kubectlUtils.getTrafficSplitAPIVersion( - kubectl - ) + trafficSplitAPIVersion = + await kubectlUtils.getTrafficSplitAPIVersion(kubectl) } return JSON.stringify({