Skip to content

interface: support BYO resource group for atm#251

Closed
zhiying-lin wants to merge 1 commit intoAzure:mainfrom
zhiying-lin:byo-rg
Closed

interface: support BYO resource group for atm#251
zhiying-lin wants to merge 1 commit intoAzure:mainfrom
zhiying-lin:byo-rg

Conversation

@zhiying-lin
Copy link
Contributor

What type of PR is this?
/kind feature
/kind api-change

What this PR does / why we need it:
Three options to support BYO rg

  1. use annotation, similar to what cloud provider supports BYO pip on the service
    • Cons: hard to the validation & hard to extend to support BYO profile in the future
  2. add new fields in the spec
    • Pros: easy to extend
    • Cons: need to define per CR
  3. create new TrafficManagerProfileClass
    • Pros: cx can define a class as the default, so later all the profile will be created using this configuration, similar to define a default ingress class.
    • Cons: too heavy as we don't have multiple cloud provider (controllers) to support this API. Usually class is used to implement the API by different controllers, often with different configuration.

Which issue(s) this PR fixes:

Fixes #

Requirements:

How has this code been tested

Special notes for your reviewer

@zhiying-lin zhiying-lin marked this pull request as draft January 24, 2025 11:05
// The Azure Resource Group of the existing resource.
// +required
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="resourceGroup is immutable"
ResourceGroup string `json:"resourceGroup"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could add name in the future to support BYO ATM profile.

type TrafficManagerProfileSpec struct {
// BringYourOwn defines the Azure Traffic Manager in your own resource group.
// +required
BringYourOwn *BringYourOwn `json:"bringYourOwn"`
Copy link

Choose a reason for hiding this comment

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

Just ResourceGroup? The bring your own noun is unnecessary, as there is no not bring your now today.

// The resource ID of the resource group to contain the Azure Traffic Manager resource corresponding to this profile. Use this option if you would like to use this profile to manage the full life cycle of the traffic manager, including creation and deletion. When this profile is created, updated, or deleted, the corresponding traffic manager with the same name will be created, updated, or deleted.

Later, we can add:

// The resource ID of the Azure Traffic Manager resource corresponding to this profile. Use this option if you would like to use this profile to update an existing traffic manager, not creating or deleting it.
ResourceId;

// If none of the above is specified, we will use the default resource group passed in as an env variable.

@zhiying-lin zhiying-lin deleted the byo-rg branch February 5, 2025 01:55
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