{AKS} az aks maintenancewindow: Add CRUD commands for peer MaintenanceWindow resources#9927
Conversation
…eWindow resources
Wires up `az aks maintenancewindow {create,show,list,update,delete,wait}`
against the existing MaintenanceWindowsOperations exposed by the vendored
azure_mgmt_preview_aks SDK (API version 2026-04-02-preview).
The MaintenanceWindow peer resource is RG-scoped and uses the same Schedule /
DailySchedule / WeeklySchedule / AbsoluteMonthlySchedule / RelativeMonthlySchedule
models that already power maintenanceConfiguration's
`aksManagedAutoUpgradeSchedule`. The construction helpers
(constructSchedule + per-type builders) are reused from maintenanceconfiguration.py
as-is; only an outer MaintenanceWindowResource wrapper is added.
LRO support: create/delete/update accept --no-wait (begin_create_or_update +
begin_delete are LRO on the SDK). `update` branches at runtime — tags-only
goes through the synchronous update_tags PATCH, schedule changes go through
the full begin_create_or_update PUT.
Requires the Microsoft.ContainerService/AKSSharedMaintenanceWindowPreview
feature to be registered on the subscription (auto-approval). Documented in
the command help.
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| aks maintenancewindow | sub group aks maintenancewindow added |
|
Hi @ibaboulfetouh, |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Azure CLI support in aks-preview for managing the new MaintenanceWindow peer ARM resource (preview API), including underlying resource construction helpers and parameter/help wiring.
Changes:
- Introduces
az aks maintenancewindowCRUD +waitcommands and client factory wiring. - Adds
maintenancewindow.pyhelper module to construct the SDK resource and detect schedule-arg presence. - Adds unit tests covering schedule construction and validation behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/aks-preview/azext_aks_preview/tests/latest/test_maintenancewindow.py | Adds unit tests for MaintenanceWindow resource construction and schedule-arg detection. |
| src/aks-preview/azext_aks_preview/maintenancewindow.py | Implements MaintenanceWindowResource construction and schedule-arg detection helpers. |
| src/aks-preview/azext_aks_preview/custom.py | Adds CLI handler implementations for maintenancewindow list/show/create/update/delete. |
| src/aks-preview/azext_aks_preview/commands.py | Registers the new aks maintenancewindow command group and operations binding. |
| src/aks-preview/azext_aks_preview/_params.py | Defines CLI arguments for maintenancewindow create/update. |
| src/aks-preview/azext_aks_preview/_help.py | Adds help text and examples for the new command group. |
| src/aks-preview/azext_aks_preview/_client_factory.py | Adds a client factory for maintenance windows. |
| src/aks-preview/HISTORY.rst | Documents the new command group in Pending release notes. |
| def aks_maintenancewindow_list( | ||
| cmd, # pylint: disable=unused-argument | ||
| client, | ||
| resource_group_name=None, | ||
| ): | ||
| if resource_group_name is None or resource_group_name == "": | ||
| return client.list_by_subscription() | ||
| return client.list(resource_group_name) |
There was a problem hiding this comment.
The vendored SDK MaintenanceWindowsOperations exposes list(resource_group_name) (returns ItemPaged[MaintenanceWindowResource]) and list_by_subscription() — confirmed in azext_aks_preview/vendored_sdks/azure_mgmt_preview_aks/operations/_operations.py line 9674. There is no list_by_resource_group method on this operations group.
Empirically confirmed by running az aks maintenancewindow list -g <rg> locally: the call resolved through to ARM and returned InvalidResourceType (RP-side; the test sub doesn't have the AKSSharedMaintenanceWindowPreview AFEC flag registered) rather than a Python AttributeError, which proves the SDK method exists and was invoked.
| if location is None: | ||
| existing = client.get(resource_group_name, maintenance_window_name) | ||
| raw_parameters["location"] = existing.location | ||
| if tags is None: | ||
| # Preserve existing tags on a schedule-only update so we don't | ||
| # accidentally clear them — PUT replaces the resource. | ||
| raw_parameters["tags"] = existing.tags |
There was a problem hiding this comment.
Good catch — the refetch-and-merge code that motivated this comment shouldn't have been there in the first place; it conflated tags-only PATCH with full PUT. Removed in 00d5c5f. The contract is now strict and documented in the help:
- tags-only → PATCH (sync,
--no-waitignored) - any schedule arg → full PUT; caller owns the entire resource body
PUT replaces the resource; tags must be re-passed if the caller wants to keep them. Updated _help.py long-summary on aks maintenancewindow update to state this explicitly. --location still defaults to the RG location when omitted (same fallback as create).
| schedule_args_present = hasAnyScheduleArg(raw_parameters) | ||
|
|
||
| # Tags-only fast path: PATCH via update_tags (sync). The CLI argument | ||
| # parser accepts --no-wait on every LRO-capable command, but the | ||
| # tags-only API is synchronous, so honor the flag by warning the user | ||
| # we ignored it rather than silently dropping it. | ||
| if not schedule_args_present: | ||
| if tags is None: | ||
| raise RequiredArgumentMissingError( | ||
| "Nothing to update. Provide --tags for a tags-only PATCH, or any of " | ||
| "--schedule-type / --interval-* / --day-of-week / --day-of-month / " | ||
| "--week-index / --duration / --utc-offset / --start-date / --start-time " | ||
| "for a full PUT." | ||
| ) |
There was a problem hiding this comment.
Addressed in 00d5c5f via option (b) — the help long-summary now states the minimum required set explicitly: --schedule-type + --start-time + --duration + per-type fields (--interval-weeks / --day-of-week / etc.).
Not pursuing option (a) (refetch + merge missing schedule fields) — that would re-introduce the same "implicit hidden state" pattern that comment #2 was flagging on tags, and would conflict with the PATCH-or-full-PUT contract the command intentionally documents.
…-PUT contract The first iteration of `aks_maintenancewindow_update` quietly refetched the existing resource on the schedule-change path to fill in `location` and preserve `tags` when omitted. That conflated tags-only PATCH with full PUT and contradicted the documented contract: - tags-only -> PATCH (sync, --no-wait ignored) - any schedule arg -> full PUT; caller owns the entire resource body Bringing the code back in line: the schedule path no longer GETs the existing resource. --location still defaults to the RG location when omitted (same fallback as create). Help text now states explicitly that PUT replaces the body and tags must be re-passed if the caller wants to keep them. Addresses Copilot review comment on PR Azure#9927 (tags-cleared edge case).
|
AKS |
| short-summary: Commands to manage MaintenanceWindow peer ARM resources. | ||
| long-summary: | | ||
| MaintenanceWindow is a resource-group-scoped ARM resource that defines a | ||
| reusable maintenance schedule. Once the `maintenanceWindowId` field is |
There was a problem hiding this comment.
nit: When we talk about it being available - do we think that's a detail customers need here?
We could just say "Users can link this resource to a pre-existing maintenanceconfiguration via the maintenanceWindowId. Multiple managedCluster maintenance configurations to a single MaintenanceWindow so the schedule lives in one place"
There was a problem hiding this comment.
Done in fc075fb. Rewrote the long-summary per your suggestion (drop the "Once available" framing in favor of a customer-focused description). New text:
"MaintenanceWindow is a resource-group-scoped ARM resource that defines a reusable maintenance schedule. Users can link this resource to a pre-existing maintenanceConfiguration via
maintenanceWindowId, so multiple managedCluster maintenance configurations share a single MaintenanceWindow and the schedule lives in one place."
| g.custom_command("list", "aks_maintenancewindow_list") | ||
| g.custom_show_command("show", "aks_maintenancewindow_show") | ||
| g.custom_command("create", "aks_maintenancewindow_create", supports_no_wait=True) | ||
| g.custom_command("update", "aks_maintenancewindow_update", supports_no_wait=True) |
There was a problem hiding this comment.
Double confirmation on delete
The command is registered with confirmation=True and the handler manually calls prompt_y_n. Users will be prompted twice. We should pick one way to confirm
There was a problem hiding this comment.
+1. confirmation=True in commands.py registration gives users a uniform --yes flag handled by the CLI core (which honors AZURE_CORE_DISABLE_CONFIRM_PROMPT and --yes consistently with every other AKS delete), while the handler-side prompt_y_n is hand-rolled. Drop the handler-side prompt and rely on confirmation=True — the yes parameter on the handler can also go away then.
There was a problem hiding this comment.
Done in fc075fb. Kept confirmation=True on the command registration (auto-injects --yes/-y via CLI core, respects AZURE_CORE_DISABLE_CONFIRM_PROMPT), dropped the handler-side prompt_y_n and the yes kwarg. Matches aks delete / aks nodepool delete. Verified with az aks maintenancewindow delete --help — single --yes -y arg now.
| # Full PUT path: the caller owns the resource body. No refetch / merge — | ||
| # this is a true PUT, mirroring `aks_maintenancewindow_create`. Default | ||
| # --location to the RG location when omitted (same fallback as create). | ||
| if location is None: |
There was a problem hiding this comment.
If the window was created in a different location, the PUT may attempt an invalid location change. Maybe we should fetch the existing resource's location?
There was a problem hiding this comment.
confirmed: ARM rejects PUTs that change the location of a TrackedResource (it's immutable post-create). Defaulting to RG location on update will hard-fail any MW that was created in a non-default region.
Two-line fix without going full read-modify-write: in aks_maintenancewindow_update, when location is None, fetch client.get(rg, name).location instead of get_rg_location. The fetch is one round-trip; cheap. Better still: combine with the not_allowed_dates fix and refactor update to read-modify-write end-to-end.
There was a problem hiding this comment.
Done in fc075fb — subsumed by the RMW refactor (see thread 3392454224 for the framing). aks_maintenancewindow_update now does client.get() first and uses existing.location for the PUT body. If --location is supplied with a different value than the existing one, we warn and keep the existing location rather than letting ARM 400 the PUT.
Test: test_location_mismatch_is_warned_not_erroring pins this — supplying --location eastus on a westus2 window emits a warning and the PUT body keeps westus2.
| operation_group="maintenance_windows", | ||
| ) | ||
|
|
||
| properties = MaintenanceWindowResourceProperties( |
There was a problem hiding this comment.
The constructed resource body never includes not_allowed_dates, existing blackout dates would get erased on schedule updates
There was a problem hiding this comment.
+1 — and the same applies to any future schedule fields beyond what constructMaintenanceWindowResource enumerates. The structural fix is either: (a) accept a --not-allowed-dates arg and add it to construct, OR (b) for an update semantics, refetch the existing MW, deep-merge user-supplied fields onto it, then PUT — which is the conventional CLI update shape (matches az aks update, az aks nodepool update, etc.). Option (b) also resolves @anushkasingh16's location-mismatch concern below in one go.
The current shape is functionally "replace" with a fast-path tags-only PATCH, named update. That's surprising — users expect update to be read-modify-write.
There was a problem hiding this comment.
Both addressed in fc075fb. Two-part fix:
-
updateis now read-modify-write — Ye's suggestion (thread 3398625216). The whole shape now matchesaz aks update/az aks nodepool update/az containerapp update(all do fetch → merge → PUT). After surveying the CLI,update_tags()is only used byaks nodepool snapshot updatebecause snapshots have no other mutable fields; there's no precedent for the "PATCH if tags-only, PUT otherwise" branching shape we had. Refetch picks upexisting.properties.not_allowed_datesfor free, so any subset-of-fields update preserves blackout dates without per-field merge code. Also future-proofs against new MW properties the RP adds. -
--config-filesupport added (MTC parity) — for the case where users do need to changenotAllowedDates. Mirrorsaks maintenanceconfiguration add --config-fileexactly: JSON-suppliedpropertiesblock replaces the existingpropertieswholesale on update. Help carries an example JSON withnotAllowedDates.
Tests: test_tags_only_update_preserves_schedule_and_dates asserts the blackout dates are preserved on a tags-only update; test_config_file_replaces_properties_keeps_tags_location asserts the JSON path works end-to-end.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| } | ||
|
|
||
|
|
||
| class TestConstructMaintenanceWindowResource(unittest.TestCase): |
There was a problem hiding this comment.
glad to see these unit test cases. In addition, could we include an end-to-end scenario test as well?
|
First-pass review on commit Replied inline on 3 of @anushkasingh16's threads (not_allowed_dates erasure, location-immutability on PUT, double-confirmation on delete) — all real bugs, all share a root cause: the Recommend: refactor One additional concern of my own:
Otherwise the CRUD shape, the |
Structural change: update is now read-modify-write.
- custom.py: aks_maintenancewindow_update refetches the existing MW via
client.get(), applies user-supplied changes onto it, then PUTs.
Matches the CLI convention used by `az aks update`,
`az aks nodepool update`, and `az containerapp update` — every
multi-field-mutable resource in the CLI follows this shape.
Side effects:
* Anushka's location-immutability bug (A3) — refetch picks up
existing.location; supplying a mismatching --location is warned
and ignored instead of letting ARM 400 the PUT.
* Anushka's not_allowed_dates silent-erasure bug (A4) — refetch
preserves blackout dates the caller didn't specify.
* Future-proofs against new MW fields without per-field merge logic.
Drops the prior tags-only PATCH fast path entirely (the SDK
update_tags() call); no precedent in the CLI for that branching
shape — snapshot is the only existing CLI user of update_tags() and
that's because snapshots are immutable apart from tags.
- custom.py + commands.py: drop double-delete-confirmation (A2 / Y2).
Keep CLI-core `confirmation=True` (auto-injects --yes/-y and
respects AZURE_CORE_DISABLE_CONFIRM_PROMPT). Drop handler-side
prompt_y_n + yes kwarg. Matches `aks delete` / `aks nodepool delete`.
- custom.py + _params.py + _help.py: add --config-file support for
create and update (mirrors `aks maintenanceconfiguration add
--config-file` and closes A4 properly). On create the JSON wholly
defines the resource; on update its `properties` block replaces the
existing properties block. The only path today to set / clear
notAllowedDates (until a dedicated --not-allowed-dates flag lands).
- _validators.py + _params.py: add validate_duration_hours enforcing
the 4-24 hour range locally so users get a clear argparse error
instead of an opaque ARM 400 (Ye's Y1).
- _help.py: rewrite group long-summary per Anushka's wording (drop
"Once available" framing; customer-focused description). Rewrite
update long-summary to describe the new RMW semantics. Add
--config-file example with notAllowedDates JSON to create + update.
- maintenancewindow.py: drop hasAnyScheduleArg helper (no callers
after the RMW refactor).
- tests/latest/test_maintenancewindow.py: +18 new test cases:
* 9 RMW path tests (tags-only preserves schedule, schedule-only
preserves tags, location-mismatch warning, no-args no-op,
get-before-put assertion, etc.)
* 3 --config-file path tests on update (replace properties keep
tags/location, --tags overrides JSON tags, must-be-dict)
* 2 --config-file path tests on create (location from JSON,
fallback to RG location)
* 8 validate_duration_hours tests (bounds inclusive/exclusive,
None is ok, negative/zero raise)
All 28 tests pass locally; existing MTC scenario tests (10) still
pass with no regressions. Sibling commands' --help renders cleanly
including the new update long-summary and --config-file arg.
|
Thanks for the structural framing — the RMW refactor lands all three of the cross-thread bugs (A2 double-confirm aside) with one change, and aligns the command shape with every other multi-field-mutable resource in the CLI. Posting the same response shape inline on each of the three threads you replied to, plus the new All five new tests ( Y1 ( |
|
LGTM! |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Related command
az aks maintenancewindowSummary
Wires up
az aks maintenancewindow {create,show,list,update,delete,wait}against the existingMaintenanceWindowsOperationsexposed by the vendoredazure_mgmt_preview_aksSDK (API version2026-04-02-preview, already in21.0.0b4).MaintenanceWindowis a resource-group-scoped ARM peer resource that defines a reusable maintenance schedule. When themaintenanceWindowIdlink field becomes available onmaintenanceConfigurations(May 2026 preview API), customers will be able to link multiple managedCluster maintenance configurations to one sharedMaintenanceWindowso the schedule lives in a single place.Approach
Hand-written commands modeled after the existing
aks maintenanceconfigurationblock. The peer-resource shape is simpler than MTC's because there's no cluster context, no legacy time-in-week schedule shape, and no per-config-name validation gymnastics — just oneMaintenanceWindowResource(Location + Tags + Properties) wrapping the sameScheduleenum/types that already poweraksManagedAutoUpgradeSchedule.Reuse, not duplication:
constructSchedule,constructDailySchedule,constructWeeklySchedule,constructAbsoluteMonthlySchedule, andconstructRelativeMonthlyScheduleinazext_aks_preview/maintenanceconfiguration.pyare imported as-is — they validate cross-arg exclusivity for all four schedule types and build theSchedulemodel. The newmaintenancewindow.pyonly adds the outerMaintenanceWindowResourcewrapper. No copy-paste; one source of truth for schedule construction.LRO support:
create/delete/updateaccept--no-wait(the SDK exposesbegin_create_or_update+begin_deleteas LRO).updatebranches at runtime — tags-only goes through the synchronousupdate_tags()PATCH; any schedule arg present routes through the fullbegin_create_or_update()PUT.Feature gate: Requires
Microsoft.ContainerService/AKSSharedMaintenanceWindowPreviewto be registered on the subscription (auto-approval). Following the established convention (e.g.EnableFIPSPreviewin_help.py), this is documented in the command help, not enforced client-side — the RP returns 403 if missing.Changes
_client_factory.pycf_maintenance_windowscommands.pymaintenance_window_sdkCliCommandType +aks maintenancewindowcommand groupmaintenancewindow.pyconstructMaintenanceWindowResource,hasAnyScheduleArgcustom.pyaks_maintenancewindow_{list,show,create,update,delete}(5 handlers)_params.pyargument_contextblocks (location auto-bound via CLI core, matching nodepool-snapshot pattern)_help.pytests/latest/test_maintenancewindow.pyHISTORY.rstTest plan
python -m unittest azext_aks_preview.tests.latest.test_maintenancewindow— 11/11 pass (4 schedule happy-paths + 4 validation cases + 3hasAnyScheduleArg)python -m unittest azext_aks_preview.tests.latest.test_maintenanceconfiguration— 10/10 still pass (no regressions from the new imports incustom.py)az aks maintenancewindow --help— group lists all 6 commands with the AFEC-flag noteaz aks maintenancewindow create --help— all 7 schedule args present,--location -lauto-bound,--no-waitauto-added, examples renderaz aks maintenancewindow update --help— documents the tags-only-vs-PUT contractGeneral Guidelines
azdev style <YOUR_EXT>locally? (localazdevnot installed;py_compileclean on all 7 touched files)python scripts/ci/test_index.py -qlocally? (no setup.py version bump in this PR; the next aks-preview release will roll all pending changes together)