-
Notifications
You must be signed in to change notification settings - Fork 17
Add command updateImage for updating a container image via revision draft
#477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
6060f1a
d80b108
b3bb7ad
f1c4700
1880dd7
2b8aa8b
c26a876
b8a1d5b
53de3bf
ce16017
822d394
8c71fbb
d309d97
ccea660
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,53 +22,56 @@ When creating or updating resources, execute steps should occupy certain priorit | |
|
|
||
| - RegistryCreateStep: 350 | ||
|
|
||
| ### 3. Image Source | ||
| ### 3. Image | ||
|
|
||
| <b>Priority Range</b>: 400 - 490 | ||
|
|
||
| #### Build Image in Azure Steps | ||
| #### General Steps | ||
| ##### Build Image in Azure Steps | ||
|
|
||
| - TarFileStep: 420 | ||
| - UploadSourceCodeStep: 430 | ||
| - RunStep: 440 | ||
| - BuildImageStep: 450 | ||
| - ContainerRegistryImageConfigureStep: 470 | ||
|
|
||
| #### Container Registry Steps | ||
| ##### Container Registry Steps | ||
|
|
||
| - ContainerRegistryImageConfigureStep: 470 | ||
|
|
||
| #### Common Steps | ||
| #### `updateImage` Steps | ||
|
|
||
| - ContainerAppUpdateStep: 480 (Todo - investigate decoupling this command from imageSource when revision draft update support is added) | ||
| - UpdateRegistryAndSecretsStep: 480 | ||
| - UpdateImageStep: 490 (revision draft) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should just call this |
||
|
|
||
| ### 4. Environment Variables | ||
| ### 4. Unallocated Space | ||
|
|
||
| <b>Priority Range</b>: 500 - 590 | ||
|
|
||
| #### Steps | ||
|
|
||
| Reserved | ||
|
|
||
| ### 5. Ingress | ||
| ### 5. Container App | ||
|
|
||
| <b>Priority Range</b>: 600 - 690 | ||
|
|
||
| #### Steps | ||
|
|
||
| - EnableIngressStep: 650 | ||
| - DisableIngressStep: 650 | ||
| - ContainerAppCreateStep: 620 | ||
| - ContainerAppUpdateStep: 650 | ||
|
|
||
| - TargetPortUpdateStep: 650 (single command only) | ||
| - ToggleIngressVisibilityStep: 650 (single command only) | ||
|
|
||
| ### 6. Container App | ||
| ### 6. Ingress | ||
|
|
||
| <b>Priority Range</b>: 700 - 790 | ||
|
|
||
| #### Steps | ||
|
|
||
| - ContainerAppCreateStep: 750 | ||
| - EnableIngressStep: 750 (update existing container app) | ||
| - DisableIngressStep: 750 (update existing container app) | ||
|
|
||
| - TargetPortUpdateStep: 750 (single command only) | ||
| - ToggleIngressVisibilityStep: 750 (single command only) | ||
|
|
||
| ### 7. Secrets | ||
|
|
||
|
|
@@ -93,8 +96,7 @@ Reserved | |
|
|
||
| #### Steps | ||
|
|
||
| - ScaleRangeUpdateStep: 1110 | ||
| - AddScaleRuleStep: 1120 | ||
| - AddScaleRuleStep: 1120 (revision draft) | ||
|
|
||
| ### 10. Unallocated Space | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| ## Commands | ||
|
|
||
| - `deployImageApi` - A command shared with the `vscode-docker` extension. It uses our old `deployImage` command flow which immediately tries to deploy the image to a container app without creating a draft. This command cannot be used to bundle template changes. | ||
|
|
||
| - `updateImage` - An ACA exclusive command that updates the container app or revision's container image via revision draft. The draft must be deployed for the changes to take effect and can be used to bundle together `Template` changes. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had a similar comment in your other PR. #476 (comment) |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ export async function getAcrCredentialsAndSecrets(context: IContainerRegistryIma | |
| const registries: RegistryCredentials[] = containerAppSettings?.registries?.filter(r => r.server !== registry.loginServer) ?? []; | ||
| registries?.push( | ||
| { | ||
| identity: '', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the dealio with this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server populates an |
||
| server: registry.loginServer, | ||
| username: username, | ||
| passwordSecretRef: passwordName | ||
|
|
@@ -45,6 +46,7 @@ export function getThirdPartyCredentialsAndSecrets(context: IContainerRegistryIm | |
| const registries: RegistryCredentials[] = containerAppSettings?.registries?.filter(r => r.server !== loginServer) ?? []; | ||
| registries?.push( | ||
| { | ||
| identity: '', | ||
| server: loginServer, | ||
| username: context.username, | ||
| passwordSecretRef | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import type { Revision } from "@azure/arm-appcontainers"; | ||
| import { nonNullProp, randomUtils } from "@microsoft/vscode-azext-utils"; | ||
| import type { Progress } from "vscode"; | ||
| import { ext } from "../../../extensionVariables"; | ||
| import type { ContainerAppItem, ContainerAppModel } from "../../../tree/ContainerAppItem"; | ||
| import type { RevisionsItemModel } from "../../../tree/revisionManagement/RevisionItem"; | ||
| import { localize } from "../../../utils/localize"; | ||
| import { getParentResourceFromItem } from "../../../utils/revisionDraftUtils"; | ||
| import { RevisionDraftUpdateBaseStep } from "../../revisionDraft/RevisionDraftUpdateBaseStep"; | ||
| import { getContainerNameForImage } from "../imageSource/containerRegistry/getContainerNameForImage"; | ||
| import type { UpdateImageContext } from "./updateImage"; | ||
|
|
||
| export class UpdateImageStep<T extends UpdateImageContext> extends RevisionDraftUpdateBaseStep<T> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mentioned it in the execute priority markdown, but I think it makes more sense to call this Maybe we should use the word save? Save tends to mean locally, or at least in place?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah let me look through some of the other steps and I'll get back to you. Maybe we can brainstorm a naming convention that works, I was thinking something similar before but didn't have any ideas that stuck. |
||
| public priority: number = 490; | ||
|
|
||
| constructor(baseItem: ContainerAppItem | RevisionsItemModel) { | ||
| super(baseItem); | ||
| } | ||
|
|
||
| public async execute(context: UpdateImageContext, progress: Progress<{ message?: string | undefined; increment?: number | undefined }>): Promise<void> { | ||
| progress.report({ message: localize('updatingImage', 'Updating image (draft)...') }); | ||
|
|
||
| this.revisionDraftTemplate.containers = []; | ||
| this.revisionDraftTemplate.containers.push({ | ||
| env: context.environmentVariables, | ||
| image: context.image, | ||
| // We need the revision draft to always show up as having unsaved changes, we can ensure this by adding a unique ID at end of the container name | ||
| name: getContainerNameForImage(nonNullProp(context, 'image')) + `-${randomUtils.getRandomHexString(5)}`, | ||
| }); | ||
|
|
||
| this.updateRevisionDraftWithTemplate(); | ||
|
|
||
| const parentResource: ContainerAppModel | Revision = getParentResourceFromItem(this.baseItem); | ||
| ext.outputChannel.appendLog(localize('updatedImage', 'Updated container app "{0}" with image "{1}" (draft).', parentResource.name, context.image)); | ||
| } | ||
|
|
||
| public shouldExecute(context: UpdateImageContext): boolean { | ||
| return !!context.containerApp && !!context.image; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import type { RegistryCredentials, Secret } from "@azure/arm-appcontainers"; | ||
| import { AzureWizardExecuteStep, nonNullProp } from "@microsoft/vscode-azext-utils"; | ||
| import * as deepEqual from "deep-eql"; | ||
| import type { Progress } from "vscode"; | ||
| import { ext } from "../../../extensionVariables"; | ||
| import { ContainerAppModel, getContainerEnvelopeWithSecrets } from "../../../tree/ContainerAppItem"; | ||
| import { localize } from "../../../utils/localize"; | ||
| import { updateContainerApp } from "../../updateContainerApp"; | ||
| import type { UpdateImageContext } from "./updateImage"; | ||
|
|
||
| export class UpdateRegistryAndSecretsStep extends AzureWizardExecuteStep<UpdateImageContext> { | ||
| public priority: number = 480; | ||
|
|
||
| public async execute(context: UpdateImageContext, progress: Progress<{ message?: string | undefined; increment?: number | undefined }>): Promise<void> { | ||
| const containerApp: ContainerAppModel = nonNullProp(context, 'containerApp'); | ||
| const containerAppEnvelope = await getContainerEnvelopeWithSecrets(context, context.subscription, containerApp); | ||
|
|
||
| // If the credentials have not changed, we can skip this update | ||
| if ( | ||
| this.areSecretsDeepEqual(containerAppEnvelope.configuration.secrets, context.secrets) && | ||
| this.areRegistriesDeepEqual(containerAppEnvelope.configuration.registries, context.registries) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| progress.report({ message: localize('configuringSecrets', 'Configuring registry secrets...') }); | ||
|
|
||
| containerAppEnvelope.configuration.secrets = context.secrets; | ||
| containerAppEnvelope.configuration.registries = context.registries; | ||
|
|
||
| await updateContainerApp(context, context.subscription, containerAppEnvelope); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| ext.outputChannel.appendLog(localize('updatedSecrets', 'Updated container app "{0}" with new registry secrets.', containerApp.name)); | ||
| } | ||
|
|
||
| public shouldExecute(context: UpdateImageContext): boolean { | ||
| return !!context.registries && !!context.secrets; | ||
| } | ||
|
|
||
| private areSecretsDeepEqual(originalSecrets: Secret[] | undefined, newSecrets: Secret[] | undefined): boolean { | ||
| originalSecrets?.sort((a, b) => sortAlphabeticallyByKey(a, b, 'name')); | ||
| newSecrets?.sort((a, b) => sortAlphabeticallyByKey(a, b, 'name')); | ||
| return deepEqual(originalSecrets, newSecrets); | ||
| } | ||
|
|
||
| private areRegistriesDeepEqual(originalRegistries: RegistryCredentials[] | undefined, newRegistries: RegistryCredentials[] | undefined): boolean { | ||
| originalRegistries?.sort((a, b) => sortAlphabeticallyByKey(a, b, 'passwordSecretRef')); | ||
| newRegistries?.sort((a, b) => sortAlphabeticallyByKey(a, b, 'passwordSecretRef')); | ||
| return deepEqual(originalRegistries, newRegistries); | ||
| } | ||
| } | ||
|
|
||
| function sortAlphabeticallyByKey<T extends Secret | RegistryCredentials>(a: T, b: T, key: keyof T): number { | ||
| if (typeof a[key] !== 'string' || typeof b[key] !== 'string') { | ||
| return 0; | ||
| } | ||
|
|
||
| const valOne = a[key] as string; | ||
| const valTwo = b[key] as string; | ||
| return valOne.localeCompare(valTwo); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.md in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import { KnownActiveRevisionsMode, Revision } from "@azure/arm-appcontainers"; | ||
| import { VerifyProvidersStep } from "@microsoft/vscode-azext-azureutils"; | ||
| import { AzureWizard, AzureWizardExecuteStep, AzureWizardPromptStep, ExecuteActivityContext, IActionContext, createSubscriptionContext } from "@microsoft/vscode-azext-utils"; | ||
| import { webProvider } from "../../../constants"; | ||
| import { ext } from "../../../extensionVariables"; | ||
| import type { ContainerAppItem, ContainerAppModel } from "../../../tree/ContainerAppItem"; | ||
| import type { RevisionDraftItem } from "../../../tree/revisionManagement/RevisionDraftItem"; | ||
| import type { RevisionItem } from "../../../tree/revisionManagement/RevisionItem"; | ||
| import { createActivityContext } from "../../../utils/activity/activityUtils"; | ||
| import { localize } from "../../../utils/localize"; | ||
| import { pickContainerApp } from "../../../utils/pickItem/pickContainerApp"; | ||
| import { pickRevision, pickRevisionDraft } from "../../../utils/pickItem/pickRevision"; | ||
| import { getParentResourceFromItem } from "../../../utils/revisionDraftUtils"; | ||
| import type { ImageSourceBaseContext } from "../imageSource/ImageSourceBaseContext"; | ||
| import { ImageSourceListStep } from "../imageSource/ImageSourceListStep"; | ||
| import { UpdateImageStep } from "./UpdateImageStep"; | ||
| import { UpdateRegistryAndSecretsStep } from "./UpdateRegistryAndSecretsStep"; | ||
|
|
||
| export type UpdateImageContext = ImageSourceBaseContext & ExecuteActivityContext; | ||
|
|
||
| export async function updateImage(context: IActionContext, node?: ContainerAppItem | RevisionItem): Promise<void> { | ||
| let item: ContainerAppItem | RevisionItem | RevisionDraftItem | undefined = node; | ||
| if (!item) { | ||
| const containerAppItem: ContainerAppItem = await pickContainerApp(context); | ||
|
|
||
| if (containerAppItem.containerApp.revisionsMode === KnownActiveRevisionsMode.Single) { | ||
| item = containerAppItem; | ||
| } else { | ||
| if (ext.revisionDraftFileSystem.doesContainerAppsItemHaveRevisionDraft(containerAppItem)) { | ||
| item = await pickRevisionDraft(context, containerAppItem); | ||
| } else { | ||
| item = await pickRevision(context, containerAppItem); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const { subscription, containerApp } = item; | ||
|
|
||
| const wizardContext: UpdateImageContext = { | ||
| ...context, | ||
| ...createSubscriptionContext(subscription), | ||
| ...await createActivityContext(), | ||
| subscription, | ||
| containerApp | ||
| }; | ||
|
|
||
| const promptSteps: AzureWizardPromptStep<UpdateImageContext>[] = [ | ||
| new ImageSourceListStep(), | ||
| ]; | ||
|
|
||
| const executeSteps: AzureWizardExecuteStep<UpdateImageContext>[] = [ | ||
| new VerifyProvidersStep([webProvider]), | ||
| new UpdateRegistryAndSecretsStep(), | ||
| new UpdateImageStep(item) | ||
| ]; | ||
|
|
||
| const parentResource: ContainerAppModel | Revision = getParentResourceFromItem(item); | ||
|
|
||
| const wizard: AzureWizard<UpdateImageContext> = new AzureWizard(wizardContext, { | ||
| title: localize('updateImage', 'Update container image for "{0}" (draft)', parentResource.name), | ||
| promptSteps, | ||
| executeSteps, | ||
| showLoadingPrompt: true | ||
| }); | ||
|
|
||
| await wizard.prompt(); | ||
| await wizard.execute(); | ||
| } |

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of find this hard to read:
I feel like a use of a divider or something could really help readability. I won't block for that, just wanted to point it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made an engineering issue