Skip to content

feat: add --skip-acceleration-wait flag to skip image acceleration po… #158

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.

Sign up for GitHub

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

Open
mozhou52 wants to merge 1 commit into
base: master
Choose a base branch
from
+167 −6
Open

feat: add --skip-acceleration-wait flag to skip image acceleration po… #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion __tests__/ut/commands/deploy/deploy_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ describe('Deploy', () => {
'async-invoke-config': undefined,
'assume-yes': false,
'skip-push': false,
'skip-acceleration-wait': false,
});

// Mock verify
Expand Down Expand Up @@ -202,7 +203,7 @@ describe('Deploy', () => {
alias: {
'assume-yes': 'y',
},
boolean: ['skip-push', 'async_invoke_config'],
boolean: ['skip-push', 'async_invoke_config', 'skip-acceleration-wait'],
});
});

Expand All @@ -213,6 +214,7 @@ describe('Deploy', () => {
'async-invoke-config': undefined,
'assume-yes': true,
'skip-push': true,
'skip-acceleration-wait': true,
});

new Deploy(mockInputs);
Expand All @@ -221,6 +223,27 @@ describe('Deploy', () => {
type: 'function',
yes: true,
skipPush: true,
skipAccelerationWait: true,
});
});

it('should pass skipAccelerationWait as undefined when not specified', () => {
(parseArgv as jest.Mock).mockReturnValue({
function: 'function',
trigger: undefined,
'async-invoke-config': undefined,
'assume-yes': true,
'skip-push': false,
'skip-acceleration-wait': false,
});

new Deploy(mockInputs);

expect(Service).toHaveBeenCalledWith(mockInputs, {
type: 'function',
yes: true,
skipPush: false,
skipAccelerationWait: false,
});
});

Expand Down
67 changes: 67 additions & 0 deletions __tests__/ut/commands/deploy/impl/function_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ describe('Service', () => {

expect(service.type).toBeUndefined();
expect(service.skipPush).toBeUndefined();
expect(service.skipAccelerationWait).toBeUndefined();
expect(service.local).toEqual({
functionName: 'test-function',
runtime: 'nodejs12',
Expand Down Expand Up @@ -237,10 +238,75 @@ describe('Service', () => {
{
slsAuto: false,
type: 'config',
skipAccelerationWait: undefined,
},
);
});

it('should pass skipAccelerationWait to deployFunction', async () => {
service = new Service(mockInputs, { ...mockOpts, skipAccelerationWait: true });
service.needDeploy = true;
Object.defineProperty(service, 'type', {
value: 'config',
writable: true,
});

const mockFcSdk = {
deployFunction: jest.fn().mockResolvedValue(undefined),
};
Object.defineProperty(service, 'fcSdk', {
value: mockFcSdk,
writable: true,
});

jest.spyOn(service as any, '_deployAuto').mockResolvedValue(undefined);
jest.spyOn(service as any, '_uploadCode').mockResolvedValue(true);

await service.run();

expect(service.fcSdk.deployFunction).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
skipAccelerationWait: true,
}),
);
});

it('should pass skipAccelerationWait=true with type=code', async () => {
service = new Service(mockInputs, { ...mockOpts, skipAccelerationWait: true });
service.needDeploy = true;
Object.defineProperty(service, 'type', {
value: 'code',
writable: true,
});

const mockFcSdk = {
deployFunction: jest.fn().mockResolvedValue(undefined),
uploadCodeToTmpOss: jest
.fn()
.mockResolvedValue({ ossBucketName: 'bucket', ossObjectName: 'object' }),
};
Object.defineProperty(service, 'fcSdk', {
value: mockFcSdk,
writable: true,
});

jest.spyOn(service as any, '_uploadCode').mockImplementation(async function (this: any) {
this.local.code = { ossBucketName: 'bucket', ossObjectName: 'object' };
return true;
});

await service.run();

expect(service.fcSdk.deployFunction).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
skipAccelerationWait: true,
type: 'code',
}),
);
});

it('should upload code when type is not config', async () => {
service = new Service(mockInputs, mockOpts);
service.needDeploy = true;
Expand Down Expand Up @@ -308,6 +374,7 @@ describe('Service', () => {
{
slsAuto: false,
type: 'code',
skipAccelerationWait: undefined,
},
);
});
Expand Down
55 changes: 55 additions & 0 deletions __tests__/ut/resources/fc/index_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,58 @@ describe('FC', () => {
expect(mockFc20230330Client.getFunction).toHaveBeenCalled();
});

it('should skip waiting when skipAccelerationWait is true for custom container runtime', async () => {
const logger = require('../../../../src/logger');

await fc.untilFunctionStateOK(mockConfig, 'CREATE', true);

expect(logger.info).toHaveBeenCalledWith(
expect.stringContaining('Skip waiting for test-image:latest optimization'),
);
expect(mockFc20230330Client.getFunction).not.toHaveBeenCalled();
});

it('should skip waiting when skipAccelerationWait is true for UPDATE reason', async () => {
const logger = require('../../../../src/logger');

await fc.untilFunctionStateOK(mockConfig, 'UPDATE', true);

expect(logger.info).toHaveBeenCalledWith(
expect.stringContaining('Skip waiting for test-image:latest optimization'),
);
expect(mockFc20230330Client.getFunction).not.toHaveBeenCalled();
});

it('should wait normally when skipAccelerationWait is false for custom container runtime', async () => {
const mockFunctionMeta = {
state: 'Active',
lastUpdateStatus: 'Success',
};

mockFc20230330Client.getFunction.mockResolvedValue({
toMap: () => ({ body: mockFunctionMeta }),
});

await fc.untilFunctionStateOK(mockConfig, 'CREATE', false);

expect(mockFc20230330Client.getFunction).toHaveBeenCalled();
});

it('should wait normally when skipAccelerationWait is undefined for custom container runtime', async () => {
const mockFunctionMeta = {
state: 'Active',
lastUpdateStatus: 'Success',
};

mockFc20230330Client.getFunction.mockResolvedValue({
toMap: () => ({ body: mockFunctionMeta }),
});

await fc.untilFunctionStateOK(mockConfig, 'CREATE');

expect(mockFc20230330Client.getFunction).toHaveBeenCalled();
});

it('should not wait for non-custom container runtime', async () => {
// Mock FC static method
const FC = require('../../../../src/resources/fc').default;
Expand Down Expand Up @@ -174,6 +226,7 @@ describe('FC', () => {
await fc.deployFunction(mockConfig, {
slsAuto: false,
type: undefined,
skipAccelerationWait: undefined,
});

expect(mockFc20230330Client.untagResources).toHaveBeenCalled();
Expand All @@ -187,6 +240,7 @@ describe('FC', () => {
fc.deployFunction(mockConfig, {
slsAuto: false,
type: undefined,
skipAccelerationWait: undefined,
}),
).rejects.toThrow('The number of tags cannot exceed 20');
});
Expand All @@ -201,6 +255,7 @@ describe('FC', () => {
fc.deployFunction(mockConfig, {
slsAuto: false,
type: undefined,
skipAccelerationWait: undefined,
}),
).rejects.toThrow('The tag keys must be unique');
});
Expand Down
4 changes: 4 additions & 0 deletions src/commands-help/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ Examples:
option: [
['-y, --assume-yes', "[Optional] Don't ask, delete directly"],
['--skip-push', '[Optional] Specify if skip automatically pushing docker container images'],
[
'--skip-acceleration-wait',
'[Optional] Specify if skip waiting for image acceleration after deploy',
],
[
"--function ['code'/'config']",
"[Optional] Only deploy function configuration or code. Use 'code' to deploy function code only, use 'config' to deploy function configuration only",
Expand Down
14 changes: 10 additions & 4 deletions src/resources/fc/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export default class FC extends FC_Client {
static isCustomRuntime = isCustomRuntime;
static replaceFunctionConfig = replaceFunctionConfig;

async untilFunctionStateOK(config: IFunction, reason: string) {
async untilFunctionStateOK(config: IFunction, reason: string, skipAccelerationWait?: boolean) {
const retryInterval = 2;
const startTime = new Date().getTime();
const calculateRetryTime = (minute: number) =>
Expand All @@ -93,6 +93,12 @@ export default class FC extends FC_Client {
const retryContainerAccelerated = FC.isCustomContainerRuntime(config.runtime);
// 部署镜像需要重试 3min, 直到达到!(State == Pending || LastUpdateStatus == InProgress)
if (retryContainerAccelerated) {
if (skipAccelerationWait) {
logger.info(
`Skip waiting for ${config.customContainerConfig.image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
);
return;
Comment on lines +96 to +100
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard customContainerConfig.image before skip-log access.

Line 98 dereferences config.customContainerConfig.image, but config-only deploy can unset customContainerConfig before this method is called (see Line 297 + Line 300). That can throw at runtime and break --skip-acceleration-wait on custom-container functions.

Proposed fix 
-      if (skipAccelerationWait) {
+      const image = _.get(config, 'customContainerConfig.image', 'custom-container image');
+      if (skipAccelerationWait) {
         logger.info(
-          `Skip waiting for ${config.customContainerConfig.image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
+          `Skip waiting for ${image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
         );
         return;
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (skipAccelerationWait) {
logger.info(
`Skip waiting for ${config.customContainerConfig.image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
);
return;
const image = config.customContainerConfig?.image ?? 'custom-container image';
if (skipAccelerationWait) {
logger.info(
`Skip waiting for ${image} optimization. The function will be available for invocation once the image acceleration process is complete.`,
);
return;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/resources/fc/index.ts` around lines 96 - 100, The skip-acceleration-wait
log dereferences config.customContainerConfig.image without ensuring
customContainerConfig exists; update the block guarded by skipAccelerationWait
(the conditional using skipAccelerationWait and logger.info) to first check that
config.customContainerConfig and config.customContainerConfig.image are defined
and only include the image in the log when present, otherwise log a fallback
message (e.g., reference to custom container image unavailable) so that invoking
skipAccelerationWait doesn't throw when customContainerConfig is unset.

}
console.log('');
if (reason === 'CREATE') {
if (isAppCenter()) {
Expand Down Expand Up @@ -183,7 +189,7 @@ export default class FC extends FC_Client {
/**
* 创建或者修改函数
*/
async deployFunction(config: IFunction, { slsAuto, type }): Promise<void> {
async deployFunction(config: IFunction, { slsAuto, type, skipAccelerationWait }): Promise<void> {
logger.debug(`Deploy function use config:\n${JSON.stringify(config, null, 2)}`);
let needUpdate = false;
let remoteConfig = null;
Expand Down Expand Up @@ -217,7 +223,7 @@ export default class FC extends FC_Client {
logger.debug(`Need create function ${config.functionName}`);
try {
await this.createFunction(config);
await this.untilFunctionStateOK(config, 'CREATE');
await this.untilFunctionStateOK(config, 'CREATE', skipAccelerationWait);
return;
} catch (ex) {
logger.debug(`Create function error: ${ex.message}`);
Expand Down Expand Up @@ -291,7 +297,7 @@ export default class FC extends FC_Client {
_.unset(config, 'customContainerConfig');
}
await this.updateFunction(config);
await this.untilFunctionStateOK(config, 'UPDATE');
await this.untilFunctionStateOK(config, 'UPDATE', skipAccelerationWait);
if (config.resourceGroupId) {
const remoteResourceGroupId = remoteConfig?.body?.resourceGroupId;
if (remoteResourceGroupId !== config.resourceGroupId) {
Expand Down
4 changes: 4 additions & 0 deletions src/subCommands/deploy/impl/function.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ interface IOpts {
type?: IType;
yes?: boolean;
skipPush?: boolean;
skipAccelerationWait?: boolean;
}

export default class Service extends Base {
readonly type?: IType;
readonly skipPush?: boolean = false;
readonly skipAccelerationWait?: boolean = false;

remote?: any;
local: IFunction;
Expand All @@ -54,6 +56,7 @@ export default class Service extends Base {

this.type = opts.type;
this.skipPush = opts.skipPush;
this.skipAccelerationWait = opts.skipAccelerationWait;
logger.debug(`deploy function type: ${this.type}`);

this.local = _.cloneDeep(inputs.props);
Expand Down Expand Up @@ -131,6 +134,7 @@ export default class Service extends Base {
await this.fcSdk.deployFunction(config, {
slsAuto: !_.isEmpty(this.createResource.sls),
type: this.type,
skipAccelerationWait: this.skipAccelerationWait,
});
return this.needDeploy;
}
Expand Down
4 changes: 3 additions & 1 deletion src/subCommands/deploy/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default class Deploy {
alias: {
'assume-yes': 'y',
},
boolean: ['skip-push', 'async_invoke_config'],
boolean: ['skip-push', 'async_invoke_config', 'skip-acceleration-wait'],
});

// TODO: 更完善的验证
Expand All @@ -53,6 +53,7 @@ export default class Deploy {
'async-invoke-config': async_invoke_config,
'assume-yes': yes,
'skip-push': skipPush,
'skip-acceleration-wait': skipAccelerationWait,
} = this.opts;
logger.debug('parse argv:');
logger.debug(this.opts);
Expand All @@ -64,6 +65,7 @@ export default class Deploy {
type,
yes,
skipPush,
skipAccelerationWait,
}); // function
}
if (deployAll || trigger) {
Expand Down
Loading