Skip to content
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

[ServiceFabric] New Az Cmdlets for Service fabric Managed app cmdlets (App, AppType, AppTypeVersion, Service) #14458

Conversation

LukeSlev
Copy link
Contributor

@LukeSlev LukeSlev commented Mar 6, 2021

Description

  • Added new cmdlets for managed applications:
    • New-AzServiceFabricManagedClusterApplication
    • Get-AzServiceFabricManagedClusterApplication
    • Set-AzServiceFabricManagedClusterApplication
    • Remove-AzServiceFabricManagedClusterApplication
    • New-AzServiceFabricManagedClusterApplicationType
    • Get-AzServiceFabricManagedClusterApplicationType
    • Set-AzServiceFabricManagedClusterApplicationType
    • Remove-AzServiceFabricManagedClusterApplicationType
    • New-AzServiceFabricManagedClusterApplicationTypeVersion
    • Get-AzServiceFabricManagedClusterApplicationTypeVersion
    • Set-AzServiceFabricManagedClusterApplicationTypeVersion
    • Remove-AzServiceFabricManagedClusterApplicationTypeVersion
    • New-AzServiceFabricManagedClusterService
    • Get-AzServiceFabricManagedClusterService
    • Set-AzServiceFabricManagedClusterService
    • Remove-AzServiceFabricManagedClusterService
  • Upgraded Managed Cluster commands to use Service Fabric Managed Cluster SDK version 1.0.0-beta.1 which uses service fabric resource provider api-version 2021-01-01-preview.

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:

@LukeSlev LukeSlev marked this pull request as draft March 6, 2021 00:24
@LukeSlev LukeSlev force-pushed the user/luslevin/managed-clusters-managed-app-cmdlets branch 3 times, most recently from baf98d6 to 5dd4762 Compare March 8, 2021 20:00
@LukeSlev LukeSlev marked this pull request as ready for review March 8, 2021 22:22
@wyunchi-ms
Copy link
Contributor

wyunchi-ms commented Mar 10, 2021

There are a lot of StaticAnalysis issue. Plesse resolve them.
And is there any design for this PR?

@LukeSlev LukeSlev force-pushed the user/luslevin/managed-clusters-managed-app-cmdlets branch from 73cca85 to 5c1fed5 Compare March 10, 2021 20:23
@a-santamaria
Copy link
Member

@wyunchi-ms the design review is here: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/845.
The breaking changes are all related to cmdlets from Service Fabric Managed Clusters which is still in preview (https://docs.microsoft.com/en-us/azure/service-fabric/overview-managed-cluster) so we are adding those to the braking changes exceptions.
Breaking changes for managed clusters :

@LukeSlev LukeSlev force-pushed the user/luslevin/managed-clusters-managed-app-cmdlets branch from 7ed74d1 to 6710824 Compare March 10, 2021 21:27
@a-santamaria a-santamaria added this to the S184 (2021-03-23) milestone Mar 10, 2021
@wyunchi-ms
Copy link
Contributor

Parameter Tags of cmdlet New-AzServiceFabricManagedClusterApplication and Set-AzServiceFabricManagedClusterApplication does not follow the enforced naming convention of using a singular noun for a parameter name.

@LukeSlev LukeSlev force-pushed the user/luslevin/managed-clusters-managed-app-cmdlets branch from 6710824 to 8285a42 Compare March 11, 2021 17:37
@LukeSlev
Copy link
Contributor Author

@wyunchi-ms I modified Tags parameter and sent an email to the PowerShell team regarding the breaking changes

@wyunchi-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

wyunchi-ms
wyunchi-ms previously approved these changes Mar 13, 2021
@wyunchi-ms
Copy link
Contributor

Please resolve the conflicts.

LukeSlev and others added 4 commits March 13, 2021 09:35
* remove .ManagedClusters from models namespace

* regenerate help

* Rename UpgradeMode to ApplicationUpgradeMode

* suppress managed clusters breaking changes as it is in preview (#2)

* Fix static analysis signature issues

* fix breaking changes exceptions (#3)

* adding missing header

* fixing breakingChangeIssues file

* add missing braking changes

* remove duplicates from braking exceptions

* fix app tests

* Change Tags to singular for application

Co-authored-by: Alfredo Santamaria Gomez <alsantam@microsoft.com>
@LukeSlev LukeSlev force-pushed the user/luslevin/managed-clusters-managed-app-cmdlets branch from 8285a42 to 935b2b2 Compare March 13, 2021 17:36
@LukeSlev
Copy link
Contributor Author

@wyunchi-ms conflicts resolved

@a-santamaria
Copy link
Member

@wyunchi-ms thanks for approving. is this going to make it to S184 (2021-03-23) release?

wyunchi-ms
wyunchi-ms previously approved these changes Mar 17, 2021
@wyunchi-ms wyunchi-ms changed the base branch from master to release-2021-03-23 March 17, 2021 05:14
@wyunchi-ms wyunchi-ms dismissed their stale review March 17, 2021 05:14

The base branch was changed.

@wyunchi-ms wyunchi-ms merged commit 4234f54 into Azure:release-2021-03-23 Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants