Update maintenanceconfiguration commands #5739

wenxuan0923 · 2023-01-09T03:49:16Z

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

az aks maintenanceconfiguration add
az aks maintenanceconfiguration update

General Guidelines

Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

My extension description/summary conforms to the Extension Summary Guidelines.

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

yonzhan · 2023-01-09T04:46:53Z

AKS

src/aks-preview/azext_aks_preview/_help.py

src/aks-preview/azext_aks_preview/_params.py

src/aks-preview/azext_aks_preview/maintenanceconfiguration.py

wenxuan0923 · 2023-01-11T01:45:51Z

Hi @FumingZhang and @zhoxing-ms, I have addressed all the comments, could you help take another look? Many thanks!!

FumingZhang

LGTM, but something related to monitoring addon broken due a recent update on azure-cli/acs. Please hold the PR until that's fixed.

FumingZhang · 2023-01-11T01:52:58Z

src/aks-preview/azext_aks_preview/_params.py

+            c.argument('duration_hours', options_list=['--duration'], type=int,
+                       help='The length of maintenance window. The value ranges from 4 to 24 hours.', required=False)


It seems that this is the only argument which has some difference between parameter and the option. Then you could omit the options_list for all other arguments.

Also you could omit required=False.

The fix (#5744) is merged. Please rebase from main then I would trigger (or you could rerun the previous one) a live test run for your newly added case. Previous one skipped the tests in aks-preview due to the breaking change in azure-cli/acs.

Thank you @FumingZhang!

I have rebased to the main branch and re-runed the live test. I also removed all the options_list for all arguments except duration_hours. When I tried to remove required=False, CLI Linter gave me failure saying I'm missing required arguments in help file, so I left them there.

FumingZhang

Live test passed.

wenxuan0923 · 2023-01-11T17:42:16Z

Hi @zhoxing-ms, could you help merge this PR? Many thanks!!

zhoxing-ms · 2023-01-12T03:44:26Z

src/aks-preview/HISTORY.rst

@@ -12,6 +12,10 @@ To release a new version, please select a new version number (usually plus 1 to
 Pending
 +++++++

+0.5.123
+++++++
+* Update command group `az aks maintenanceconfiguration` to enable the creation of *aksManagedAutoUpgradeSchedule* and *aksManagedNodeOSUpgradeSchedule*. 


aksManagedAutoUpgradeSchedule and aksManagedNodeOSUpgradeSchedule

Could we describe these two features in detail in the history notes, otherwise their meaning is easily confusing?

Thanks for your feedback, I have updated the details!

zhoxing-ms · 2023-01-12T03:47:17Z

src/aks-preview/azext_aks_preview/_help.py

+          short-summary: Specifies the number of days between each set of occurrences for daily schedule type.
+        - name: --interval-weeks


Suggested change

short-summary: Specifies the number of days between each set of occurrences for daily schedule type.

- name: --interval-weeks

short-summary: Specify the number of days between each set of occurrences for daily schedule type.

- name: --interval-weeks

Please use the first-person voice in the help message, and other places are similar. Please modify these voices uniformly

zhoxing-ms · 2023-01-12T03:52:30Z

src/aks-preview/azext_aks_preview/_params.py

+            c.argument('start_hour', type=int, required=False, options_list=[
+                       '--start-hour'], help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')


Suggested change

c.argument('start_hour', type=int, required=False, options_list=[

'--start-hour'], help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

c.argument('start_hour', type=int, help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

Because required=False and options_list=['--start-hour'] is the default setting, so there is no need to explicitly repeat the definition

All required=False and unwanted options_list have been removed

zhoxing-ms · 2023-01-12T03:55:20Z

src/aks-preview/azext_aks_preview/_params.py

+            c.argument('schedule_type', arg_type=get_enum_type(schedule_types),
+                       help='Schedule type for non-default maintenance configuration.', required=False)
+            c.argument('interval_days', type=int, help='The number of days between each set of occurrences for Daily schedule.', required=False)
+            c.argument('interval_weeks', type=int, help='The number of weeks between each set of occurrences for Weekly schedule.', required=False)
+            c.argument('interval_months', type=int, help='The number of months between each set of occurrences for AbsoluteMonthly or RelativeMonthly schedule.', required=False)
+            c.argument('day_of_week', help='Specifies on which day of the week the maintenance occurs for Weekly or RelativeMonthly schedule.', required=False)
+            c.argument('day_of_month', help='Specifies on which date of the month the maintenance occurs for AbsoluteMonthly schedule.', required=False)
+            c.argument('week_index', arg_type=get_enum_type(week_indexes), required=False,
+                       help='Specifies on which instance of the weekday specified in --day-of-week the maintenance occurs for RelativeMonthly schedule.')
+            c.argument('duration_hours', options_list=['--duration'], type=int, required=False,
+                       help='The length of maintenance window. The value ranges from 4 to 24 hours.')
+            c.argument('utc_offset', validator=validate_utc_offset, required=False,
+                       help='The UTC offset in format +/-HH:mm. e.g. -08:00 or +05:30.')
+            c.argument('start_date', validator=validate_start_date, required=False,
+                       help='The date the maintenance window activates. e.g. 2023-01-01.')
+            c.argument('start_time', validator=validate_start_time, required=False,
+                       help='The start time of the maintenance window. e.g. 09:30.')


required=False

Same as above

Update: I have to add required=False back in order to pass Linter and Live tests T^T . I posted the question in the Azure CLI channel: https://teams.microsoft.com/l/message/19:[email protected]/1673502442579?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=9a661795-1b01-4a5b-9dd5-be571422334c&parentMessageId=1673502442579&teamName=Azure%20CLIs%20partners&channelName=Azure%20CLI&createdTime=1673502442579&allowXTenantAccess=false

Update again: required=False are removed!

src/aks-preview/azext_aks_preview/custom.py

wenxuan0923 · 2023-01-13T02:02:51Z

Hi @zhoxing-ms, I have addresses you comments. Could you help take another look? Thanks in advance!!

zhoxing-ms · 2023-01-13T03:30:44Z

src/aks-preview/azext_aks_preview/_params.py

+            c.argument('day_of_week', help='Specifies on which day of the week the maintenance occurs for Weekly or RelativeMonthly schedule.')
+            c.argument('day_of_month', help='Specifies on which date of the month the maintenance occurs for AbsoluteMonthly schedule.')
+            c.argument('week_index', arg_type=get_enum_type(week_indexes),
+                       help='Specifies on which instance of the weekday specified in --day-of-week the maintenance occurs for RelativeMonthly schedule.')


Suggested change

c.argument('day_of_week', help='Specifies on which day of the week the maintenance occurs for Weekly or RelativeMonthly schedule.')

c.argument('day_of_month', help='Specifies on which date of the month the maintenance occurs for AbsoluteMonthly schedule.')

c.argument('week_index', arg_type=get_enum_type(week_indexes),

help='Specifies on which instance of the weekday specified in --day-of-week the maintenance occurs for RelativeMonthly schedule.')

c.argument('day_of_week', help='Specify on which day of the week the maintenance occurs for Weekly or RelativeMonthly schedule.')

c.argument('day_of_month', help='Specify on which date of the month the maintenance occurs for AbsoluteMonthly schedule.')

c.argument('week_index', arg_type=get_enum_type(week_indexes),

help='Specify on which instance of the weekday specified in --day-of-week the maintenance occurs for RelativeMonthly schedule.')

Ah sorry I missed these. They have been fixed!

zhoxing-ms · 2023-01-13T03:31:50Z

src/aks-preview/azext_aks_preview/_params.py

+            c.argument('weekday', help='weekday on which maintenance can happen. e.g. Monday')
+            c.argument('start_hour', type=int, help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')


Suggested change

c.argument('weekday', help='weekday on which maintenance can happen. e.g. Monday')

c.argument('start_hour', type=int, help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

c.argument('weekday', help='Weekday on which maintenance can happen. e.g. Monday')

c.argument('start_hour', type=int, help='Maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

FumingZhang

Please use mocked time instead of real time.

FumingZhang · 2023-01-13T06:17:16Z

src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py

+            'auto_upgrade_config_name': 'aksManagedAutoUpgradeSchedule',
+            'node_os_upgrade_config_name': 'aksManagedNodeOSUpgradeSchedule',
+            'ssh_key_value': self.generate_ssh_keys(),
+            'future_date': (datetime.datetime.now() + datetime.timedelta(days=10)).strftime("%Y-%m-%d")


Just wondered why the recording test failed based on previous commit.

It's not a good idea to use real time instead of mocked time in e2e test, which would fail the recording test later.

I have modified the test to use a fixed future date. Thanks for the explanation!

zhoxing-ms · 2023-01-16T03:12:05Z

@wenxuan0923 Could you please resolve these conflicts?

FumingZhang

Queue another live test to do the final verification. Looks good!

wenxuan0923 requested review from andyliuliming and FumingZhang as code owners January 9, 2023 03:49

yonzhan requested review from zhoxing-ms and yanzhudd January 9, 2023 04:47

yonzhan assigned zhoxing-ms Jan 9, 2023

yonzhan added this to the Jan 2023 (2023-02-07) milestone Jan 9, 2023

FumingZhang reviewed Jan 9, 2023

View reviewed changes

wenxuan0923 added 3 commits January 9, 2023 15:36

Resolve merge conflicts

b02011d

Fx indent

2f47d6a

Missed one

2702d72

FumingZhang reviewed Jan 11, 2023

View reviewed changes

wenxuan0923 added 2 commits January 10, 2023 19:02

Rebase main

efe88a0

Remove options list

e812c9a

FumingZhang approved these changes Jan 11, 2023

View reviewed changes

zhoxing-ms reviewed Jan 12, 2023

View reviewed changes

wenxuan0923 added 2 commits January 11, 2023 21:08

Address comments

017c732

Fx

104b999

FumingZhang reviewed Jan 12, 2023

View reviewed changes

src/aks-preview/azext_aks_preview/custom.py Outdated Show resolved Hide resolved

Add default value to all optional arguments

ad39561

zhoxing-ms reviewed Jan 13, 2023

View reviewed changes

wenxuan0923 added 2 commits January 12, 2023 21:21

Fx help msg

a31193e

Rerun live test

48c3b70

FumingZhang requested changes Jan 13, 2023

View reviewed changes

Use fixed future date

114283f

zhoxing-ms approved these changes Jan 16, 2023

View reviewed changes

Merge branch 'main' into wenx/add-new-op

12e601e

FumingZhang approved these changes Jan 16, 2023

View reviewed changes

zhoxing-ms merged commit b95c721 into Azure:main Jan 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update maintenanceconfiguration commands #5739

Update maintenanceconfiguration commands #5739

wenxuan0923 commented Jan 9, 2023 •

edited

Loading

yonzhan commented Jan 9, 2023

wenxuan0923 commented Jan 11, 2023

FumingZhang left a comment

FumingZhang Jan 11, 2023

FumingZhang Jan 11, 2023

wenxuan0923 Jan 11, 2023

FumingZhang left a comment

wenxuan0923 commented Jan 11, 2023

zhoxing-ms Jan 12, 2023

wenxuan0923 Jan 12, 2023

zhoxing-ms Jan 12, 2023

wenxuan0923 Jan 12, 2023

zhoxing-ms Jan 12, 2023 •

edited

Loading

wenxuan0923 Jan 12, 2023

zhoxing-ms Jan 12, 2023

wenxuan0923 Jan 12, 2023

wenxuan0923 Jan 12, 2023

wenxuan0923 Jan 12, 2023

wenxuan0923 commented Jan 13, 2023

zhoxing-ms Jan 13, 2023

wenxuan0923 Jan 13, 2023

zhoxing-ms Jan 13, 2023

wenxuan0923 Jan 13, 2023

FumingZhang left a comment

FumingZhang Jan 13, 2023

wenxuan0923 Jan 13, 2023

zhoxing-ms commented Jan 16, 2023

FumingZhang left a comment

		c.argument('duration_hours', options_list=['--duration'], type=int,
		help='The length of maintenance window. The value ranges from 4 to 24 hours.', required=False)

		short-summary: Specifies the number of days between each set of occurrences for daily schedule type.
		- name: --interval-weeks

		c.argument('start_hour', type=int, required=False, options_list=[
		'--start-hour'], help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

	c.argument('start_hour', type=int, required=False, options_list=[
	'--start-hour'], help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')
	c.argument('start_hour', type=int, help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

		c.argument('weekday', help='weekday on which maintenance can happen. e.g. Monday')
		c.argument('start_hour', type=int, help='maintenance start hour of 1 hour window on the weekday. e.g. 1 means 1:00am - 2:00am')

Update maintenanceconfiguration commands #5739

Update maintenanceconfiguration commands #5739

Conversation

wenxuan0923 commented Jan 9, 2023 • edited Loading

Related command

General Guidelines

About Extension Publish

yonzhan commented Jan 9, 2023

wenxuan0923 commented Jan 11, 2023

FumingZhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FumingZhang left a comment

Choose a reason for hiding this comment

wenxuan0923 commented Jan 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhoxing-ms Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenxuan0923 commented Jan 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FumingZhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhoxing-ms commented Jan 16, 2023

FumingZhang left a comment

Choose a reason for hiding this comment

wenxuan0923 commented Jan 9, 2023 •

edited

Loading

zhoxing-ms Jan 12, 2023 •

edited

Loading