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

Restructure KubernetesConfiguration directories to a suite of services #31523

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

dipti-pai
Copy link
Member

@dipti-pai dipti-pai commented Nov 13, 2024

This PR splits the Kubernetes Configuration Services into a service group with 5 services. The swaggers for each service group has no new changes except FluxConfiguration Service which includes addition of a new provider field. ARM changes were reviewed by ARM team and signed off in PR #30956 . The service group restructuring has been discussed with the Azure Breaking Changes Board.

These are the 5 services -

  • Flux Configuration Service
    -- new API version: stable 2024-11-01 version
    -- base version specification/kubernetesconfiguration/resource-manager/Microsoft.KubernetesConfiguration/preview/2024-04-01-preview/fluxconfiguration.json

  • Extensions Service
    -- new API version: stable 2024-11-02 and preview 2024-11-02-preview version
    -- base API version:
    -- extensions: specification/kubernetesconfiguration/resource-manager/Microsoft.KubernetesConfiguration/stable/2023-05-01
    -- extensionTypes: specification/kubernetesconfiguration/resource-manager/Microsoft.KubernetesConfiguration/preview/2023-05-01-preview/extensionTypes.json

  • Private Link Scopes Service
    -- new API version: preview 2024-11-01-preview
    -- base API version: specification/kubernetesconfiguration/resource-manager/Microsoft.KubernetesConfiguration/preview/2022-04-02-preview/privateLinkScopes.json

  • Operations Service
    -- new API version: stable 2024-11-01
    -- base API version: specification/kubernetesconfiguration/resource-manager/Microsoft.KubernetesConfiguration/stable/2023-05-01/operations.json

  • Source Control Configuration (stable version of retired service)
    -- new API version: stable 2024-11-01
    -- base API version: specification/kubernetesconfiguration/resource-manager/Microsoft.KubernetesConfiguration/stable/2023-05-01/kubernetesconfiguration.json

ARM (Control Plane) API Specification Update Pull Request

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

spec_pr_review_workflow_diagram

Purpose of this PR

What's the purpose of this PR? Check the specific option that applies. This is mandatory!

  • New resource provider.
  • [] New API version for an existing resource provider. (If API spec is not defined in TypeSpec, the PR should have been created in adherence to OpenAPI specs PR creation guidance).
  • Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
  • Update existing version to fix OpenAPI spec quality issues in S360.
  • Convert existing OpenAPI spec to TypeSpec spec (do not combine this with implementing changes for a new API version).
  • Other, please clarify: Restructuring KubernetesConfiguration directories into suite of services with the new structure defined here. Proposal for the directory structure in this PR have been discussed and reviewed with the Azure Breaking Changes Board.

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

  • I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
  • I have reviewed following Resource Provider guidelines, including
    ARM resource provider contract and
    REST guidelines (estimated time: 4 hours).
    I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.

Additional information

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
suppressions guide to get approval.

Getting help

  • First, please carefully read through this PR description, from top to bottom. Please fill out the Purpose of this PR and Due diligence checklist.
  • If you don't have permissions to remove or add labels to the PR, request write access per aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories
  • To understand what you must do next to merge this PR, see the Next Steps to Merge comment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.
  • For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure
    and https://aka.ms/ci-fix.
  • For help with ARM review (PR workflow diagram Step 2), see https://aka.ms/azsdk/pr-arm-review.
  • If the PR CI checks appear to be stuck in queued state, please add a comment with contents /azp run.
    This should result in a new comment denoting a PR validation pipeline has started and the checks should be updated after few minutes.
  • If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.

Copy link

openapi-pipeline-app bot commented Nov 13, 2024

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ This is the public specs repo main branch which is not intended for iterative development.
    You must acknowledge that you understand that after this PR is merged, you won't be able to stop your changes from being published to Azure customers.
    If this is what you intend, add PublishToCustomers label to your PR.
    Otherwise, retarget this PR onto a feature branch, i.e. with prefix release- (see aka.ms/azsdk/api-versions#release--branches).
  • ❌ This PR is in purview of the ARM review (label: ARMReview). This PR must get ARMSignedOff label from an ARM reviewer.
    This PR is awaiting ARM reviewer feedback (label: WaitForARMFeedback).
    To learn when this PR will get reviewed, see ARM review queue at aka.ms/azsdk/pr-arm-review
    For details of the ARM review, see aka.ms/azsdk/pr-arm-review
  • ❌ The required check named Swagger LintDiff has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide

Copy link

openapi-pipeline-app bot commented Nov 13, 2024

Generated ApiView

Language Package Name ApiView Link
Go sdk/resourcemanager/kubernetesconfiguration/armextensions https://apiview.dev/Assemblies/Review/2d133d06397b482bb3e311b6a7508fa7?revisionId=f8750ac688ab4c999fc9d15ded694545
Go sdk/resourcemanager/kubernetesconfiguration/armextensiontypes https://apiview.dev/Assemblies/Review/777b52bb4dd546448cf9a86c8e418949?revisionId=be35a07d866940d4820776a4a3d970dc
Go sdk/resourcemanager/kubernetesconfiguration/armfluxconfigurations https://apiview.dev/Assemblies/Review/1c53c9dd87ce41848d2b91d61efb99fe?revisionId=59a76642578a4fa9a12c31298b90af1d
Go sdk/resourcemanager/kubernetesconfiguration/armkubernetesconfiguration https://apiview.dev/Assemblies/Review/a54df2a0c1e54238a5d668c54b45f1ef?revisionId=2b0d09fe77144a94b49cc4fdac6cd238
Go sdk/resourcemanager/kubernetesconfiguration/armoperations https://apiview.dev/Assemblies/Review/4f4bae2e08f34254973bc3f9bdb71ed5?revisionId=ecfb42b8158940ada4fdc58f7fa94713
Go sdk/resourcemanager/kubernetesconfiguration/armprivatelinkscopes https://apiview.dev/Assemblies/Review/0c936db75a0a4853af38a1773a9719d4?revisionId=e8de151209054b26894ccf03160902c3
Go sdk/resourcemanager/kubernetesconfiguration/armsourcecontrolconfigurations https://apiview.dev/Assemblies/Review/a0431414ff28456dbd7aef925852e033?revisionId=34bcb93035fa4e99859ef229df8a69f7
Java azure-resourcemanager-kubernetesconfiguration https://apiview.dev/Assemblies/Review/b32f2ce6cb0c483b98818ba71fed6114?revisionId=c7af07f8863c48948713c0274cf80480
Python azure-mgmt-kubernetesconfiguration Create ApiView timeout. Package is too large and we cannot create ApiView for it.
JavaScript @azure/arm-kubernetesconfiguration-extensions https://apiview.dev/Assemblies/Review/9c1233c6c7174ab8b1a60302c135cdad?revisionId=c887c9ca3f2942bf834643ac68f147a0
JavaScript @azure/arm-kubernetesconfiguration-extensiontypes https://apiview.dev/Assemblies/Review/99b1cb4b317e437aa0fec812b7ef17f2?revisionId=e701708a1b9f45a797d606723fe1aaf2
JavaScript @azure/arm-kubernetesconfiguration-fluxconfigurations https://apiview.dev/Assemblies/Review/e087b8b9cb6c4323ae0da74b8806c3e3?revisionId=9a5fcd6c14ec460eba6d964f9554d8c1
JavaScript @azure/arm-kubernetesconfiguration-operations https://apiview.dev/Assemblies/Review/76f0e27f304e42a99bef81f2f6131771?revisionId=8ac17f369db24ab9925b1c3e8c55f1c3
JavaScript @azure/arm-kubernetesconfiguration-privatelinkscopes https://apiview.dev/Assemblies/Review/705c717f08294c89bb181632a36b2e7e?revisionId=6a57bdd33ade410ab14e967c733927cf
JavaScript @azure/arm-kubernetesconfiguration-sourcecontrolconfigurations https://apiview.dev/Assemblies/Review/1a3bd804eb2a436b918a767f6e56be3a?revisionId=60999dbcaee84736a9025c69e5ab2b8c
JavaScript @azure/arm-kubernetesconfiguration https://apiview.dev/Assemblies/Review/9c38fe14e2c0467c9767f42827a62a52?revisionId=239d86830fc348698ce6020adc598e97

@AzureRestAPISpecReview AzureRestAPISpecReview added ARMReview new-api-version resource-manager SuppressionReviewRequired WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Nov 13, 2024
…up consisting of 5 services -

- Flux Configuration Service (stable 2024-11-01 version)
- Extensions Service (stable 2024-11-02 and preview 2024-11-02-preview version)
- Private Link Scopes Service (preview 2024-11-01-preview)
- Operations Service (stable 2024-11-01)
- Source Control Configuration (stable version of retired service)

FluxConfiguration Service includes addition of a new provider field.

Signed-off-by: Dipti Pai <diptipai@microsoft.com>
Signed-off-by: Dipti Pai <diptipai@microsoft.com>
@razvanbadea-msft
Copy link
Contributor

@dipti-pai what is the base version i can use for comparing the files? why there are different versions for each service?

@razvanbadea-msft
Copy link
Contributor

see also the errors from the required modelvalidation, semanticvalidation, lintdiff checks

Signed-off-by: Dipti Pai <diptipai@microsoft.com>
@AzureRestAPISpecReview AzureRestAPISpecReview removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Nov 13, 2024
@JeffreyRichter
Copy link
Member

I agree, let's go shorter: armextensiontypes

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Nov 21, 2024

i don't know how other language will deal with this case. loop in ower: @weidongxu-microsoft @msyyc @qiaozha @ArthurMa1978.

My main concern is what's the fate of the current GAed kubernetesconfiguration lib.
https://azure.github.io/azure-sdk/releases/latest/?search=kubernetesconfiguration

If we discontinue it, I am not sure who need to be involved to discus this case, of we release a GA lib, then abandon the lib.

@tadelesh @weidongxu-microsoft, This will be a one-time breaking change for the SDK consumers. The changes made here have been discussed with and are as per the guidance provided by the Breaking Change Board, API Stewardship Board, & SDK Architecture Board. I have an issue here for review of the names with the SDK board. Could you chime in with your thoughts there ? Thanks so much!

@dipti-pai
Sure, if it already passes all parties, then you are good to break.

But currently this namespace review item seems having issue with Java
image

  1. I haven't seen any package name with a dot azure-resourcemanager-kubernetesconfiguration.fluxconfigurations in our repo. Should it be a hyphen -?
  2. Would the azure-resourcemanager-kubernetesconfiguration.operations package have only 1 API (list operations)? If so, I don't think we are going to generate this as an SDK.

PS: after the namespace get approved, Java dev will help with a one-time config in SDK repo to get your automation green

@dipti-pai dipti-pai removed the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Nov 21, 2024
@openapi-pipeline-app openapi-pipeline-app bot added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Nov 21, 2024
@dipti-pai
Copy link
Member Author

dipti-pai commented Nov 21, 2024

@weidongxu-microsoft

  1. I haven't seen any package name with a dot azure-resourcemanager-kubernetesconfiguration.fluxconfigurations in our repo. Should it be a hyphen -?
    [Dipti] Yes, this was a typo, I fixed it. Thanks so much!
  1. Would the azure-resourcemanager-kubernetesconfiguration.operations package have only 1 API (list operations)? If so, I don't think we are going to generate this as an SDK.
    [Dipti] Yes, operations does have only one API but it returns a list of APIs from all services so it did not belong to any one service. I have asked for recommendations from the ARM team regarding how other RPs that are splitting their services into service group are handling operations.

PS: after the namespace get approved, Java dev will help with a one-time config in SDK repo to get your automation green
[Dipti] Thank you so much! Really appreciate you and your team's help with this.

Signed-off-by: Dipti Pai <diptipai@microsoft.com>
Signed-off-by: Dipti Pai <diptipai@microsoft.com>
This was referenced Nov 21, 2024
Signed-off-by: Dipti Pai <diptipai@microsoft.com>
@ArthurMa1978
Copy link
Member

I have a few questions regarding this change:

Operation Method: The operation method is universally applicable to all ARM RPs. Following the principle that "services can and will version independently from each other," each service should maintain its own operation method for retrieving or listing operations. Therefore, this change incorrectly separates it into an independent service that returns operations for all services collectively.

Source Control Configuration: If the Source Control Configuration is retired, it should not be included in the new spec. Can we remove it?

Extension Service*: Why is the Extension Service split into two namespaces? Would it be possible to combine Extensions and ExtensionTypes into a single Extensions namespace?

@tadelesh
Copy link
Member

@JeffreyRichter @jhendrixMSFT then this service will have several separated module packages. each package contain part of the service's resources, which i doubt if it follows our guideline? (especially for armoperations, which is weird).

@JeffreyRichter
Copy link
Member

What guidelines are you referring to?
An RP is not a "service". It is a container for 1 or more services which is what we're doing here: we're formally splitting the rp operations into cleanly versioning services for long-term sustainability. We will be doing this with RPs as well as time goes on.

This will require the 1-time break of obsoleting monolithic sdk packages in all languages and introducing new sdk packages (one per service, not RP). For years now the sdk team has obsoleted sdk packages when a new version ships our when a service retires; we'd follow the same process to retire the old monolithic sdk package here. Lori Farleigh is the sdk pm owning this process and she aware of this specific rp and what is happening here.

Arthur raises some good points. Maybe we do not need an sdk (or swaggers) for the retiring service and customers can just continue to use the old monolithic sdk as new features should not be introduced. @dipti-pai, please comment.

@dipti-pai: doors it make sense to merge the extension operations together into a single service?

The "operations service" is a problem. I think I need more education about it. I'm inferring from this conversation that each RP has 1 operations Operation and this returns a collection of the RPs operations? Who invokes this Operation Operation and why do they income it? Do we need it at all? Maybe we can just get rid of it in swagger/sdk?

If we can't get rid of it, then how does this Operation version: whenever there is any change to the rp? And, if it returns all of the RPs operations, doors it include the api-version for each Operation? Can I get a link to the rest docs for the Operation Operation?

@dipti-pai
Copy link
Member Author

dipti-pai commented Nov 21, 2024

@JeffreyRichter @jhendrixMSFT @ArthurMa1978 @tadelesh Thank you for all the discussions.

  • SourceControlConfigurations is in a deprecation path from RP perspective and we do not expect any new features. If there is no requirement from the Azure Service Versioning guidelines to make this part of the new directory structure, our team is ok skipping this.

  • Extension and ExtensionTypes serve different personas. ExtensionTypes is a discovery API that helps the customer understand which extensions are available in which regions. We do not expect extensionTypes to be used in any automations/devops workflows. Extensions is the main API that most customers will be interacting with/including in their development workflows to manage the lifecycle of extensions. Our team discussed the modelling of these two APIs and decided they should be separate.

  • Operations - The reason to keep operations separate was because it is an ARM construct that returns all operations supported by an RP. We are open to suggestions here regarding best practices followed by other teams.

Please let me know if a call would help to close on these considerations. Thanks.

@JeffreyRichter
Copy link
Member

JeffreyRichter commented Nov 21, 2024 via email

@tadelesh
Copy link
Member

tadelesh commented Nov 22, 2024

the guideline i mentioned is one service should only have one sdk package (for go is one module). then my concerns are:

  1. i believe customer think in terms of service, so the service here means an azure service. to me, an azure service is something like virtual machine, vmss, etc. but here the private link scopes, extensions, etc., is hard to recognize them as service without rp concept.
  2. just as we all mentioned, operations list is an operation exist in all rps. since it is for rp, it should not exist in any service sdk i believe.
  3. for go module path, if we want to keep it as sdk/resourcemanager/kubernetesconfiguration/armextensiontypes, then what should customer consider of the kubernetesconfiguration part, rp or service group?

@JeffreyRichter
Copy link
Member

JeffreyRichter commented Nov 22, 2024 via email

@dipti-pai
Copy link
Member Author

dipti-pai commented Nov 22, 2024

@JeffreyRichter

I asked questions about operations operation which no one is answering. Who calls this operation and why? How does the rp decide what api-version it has and how does this version over time? When it returns other operations supported by the rp, what api-version values are returned for those operations? Can I get a link to the rest docs for this operations operation?

Sharing the information I found -

  • Operations is a discovery API that is API Version Neutral that all resource providers must support.
  • The Operations API versions anytime the RP adds a new operation that it supports
  • It does not return the versions of the operations that the RP supports.

This doc explains the details.

@JeffreyRichter
Copy link
Member

I read the doc and I also found this: https://learn.microsoft.com/en-us/rest/api/kubernetesconfiguration/operations/list?view=rest-kubernetesconfiguration-2023-05-01&tabs=HTTP

The doc is interesting because it says this:
As part of the management experience, clients (e.g. portal / CLI / powershell) need an ability to discover the available operations for a particular resource provider. The set of operations should include both registered and non-registered types (e.g. Virtual Machines and Disks).

This API is unique in that it is not scoped to a subscription – it is considered to be tenant-wide and should not vary based on particular subscriptions.

I take away a few things from this:
Clients need the ability to discover the available operations; I think this is just for listing to the operations to a customer and not for programmatic usage of the results.
Since the response list of operations doesn't include api-version values, the results are not usable anyway and no one could ever assume that the apu-version value passed into this operation would return operations for the same api-version since rp operations never made this requirement or worked this way. Hence, I conclude that the operations operation is practically useless.

Since the Operations Operation is unique in that it doesn't take a subscription and I also guess that authentication is not required, this method should be on its on sdk client object anyway and not a method of a client that does require a subscription/authentication. This client could be its own separate sdk package and I suspect that practically no one would ever take a dependency on it due to this operation's uselessness.

Therefore, I say that all RPs should have their own operations service with it's own docs and sdk package that will almost never be used. I guess the api-version values it accepts matches all values supported by any of the rp's services. This is just another example of how the end to end versioning for customers was never thought through, not documented well enough, and how different service teams probably implement it differently making the behavior not reliable for customers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMReview BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. BreakingChange-Go-Sdk BreakingChange-JavaScript-Sdk BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required new-api-version resource-manager SuppressionReviewRequired WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.