-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
azurerm_kubernetes_fleet_manager
- Add support for hub_profile
property
#28365
base: main
Are you sure you want to change the base?
azurerm_kubernetes_fleet_manager
- Add support for hub_profile
property
#28365
Conversation
}) | ||
} | ||
|
||
func TestAccKubernetesFleetManager_update(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this test case to verify if the tags
can be updated. You can create a update
function to provide the config with updated tags
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var model KubernetesFleetManagerModel | ||
if err := metadata.Decode(&model); err != nil { | ||
return fmt.Errorf("decoding: %+v", err) | ||
} | ||
|
||
client := metadata.Client.ContainerService.V20231015.Fleets | ||
subscriptionId := metadata.Client.Account.SubscriptionId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please be mindful of the ordering of calls here, boilerplate code should follow the same pattern and ordering as the rest of the provider.
var model KubernetesFleetManagerModel | |
if err := metadata.Decode(&model); err != nil { | |
return fmt.Errorf("decoding: %+v", err) | |
} | |
client := metadata.Client.ContainerService.V20231015.Fleets | |
subscriptionId := metadata.Client.Account.SubscriptionId | |
client := metadata.Client.ContainerService.V20231015.Fleets | |
subscriptionId := metadata.Client.Account.SubscriptionId | |
var model KubernetesFleetManagerModel | |
if err := metadata.Decode(&model); err != nil { | |
return fmt.Errorf("decoding: %+v", err) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
if !response.WasNotFound(existing.HttpResponse) { | ||
return metadata.ResourceRequiresImport(r.ResourceType(), id) | ||
} | ||
|
||
var payload fleets.Fleet | ||
r.mapKubernetesFleetManagerResourceSchemaToFleet(config, &payload) | ||
fleetResource := &fleets.Fleet{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is usually called params
or payload
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
output.Tags = tags.Flatten(input.Tags) | ||
func flattenFleetHubProfileModel(input *fleets.FleetHubProfile) []FleetHubProfileModel { | ||
if input == nil { | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should return an empty hub profile model here
return nil | |
return []FleetHubProfileModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
This PR is being labeled as "stale" because it has not been updated for 30 or more days. If this PR is still valid, please remove the "stale" label. If this PR is blocked, please add it to the "Blocked" milestone. If you need some help completing this PR, please leave a comment letting us know. Thank you! |
@stephybun Hi, thanks for review, updated the fix. Ready for rereview. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hqhqhqhqhqhqhqhqhqhqhq could you resolve the merge conflict and the comment I left in-line? Once that's done this should be good to go.
properties := resp.Model | ||
if resp.Model == nil { | ||
return fmt.Errorf("retrieving %s: `model` was nil", *id) | ||
} | ||
if resp.Model.Properties == nil { | ||
return fmt.Errorf("retrieving %s: `properties` was nil", *id) | ||
} | ||
|
||
if metadata.ResourceData.HasChange("tags") { | ||
properties.Tags = pointer.To(model.Tags) | ||
} | ||
|
||
if err := client.CreateOrUpdateThenPoll(ctx, *id, *properties, fleets.DefaultCreateOrUpdateOperationOptions()); err != nil { | ||
return fmt.Errorf("updating %s: %+v", *id, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not patching anything into properties so there's no need to nil check this.
properties := resp.Model | |
if resp.Model == nil { | |
return fmt.Errorf("retrieving %s: `model` was nil", *id) | |
} | |
if resp.Model.Properties == nil { | |
return fmt.Errorf("retrieving %s: `properties` was nil", *id) | |
} | |
if metadata.ResourceData.HasChange("tags") { | |
properties.Tags = pointer.To(model.Tags) | |
} | |
if err := client.CreateOrUpdateThenPoll(ctx, *id, *properties, fleets.DefaultCreateOrUpdateOperationOptions()); err != nil { | |
return fmt.Errorf("updating %s: %+v", *id, err) | |
if resp.Model == nil { | |
return fmt.Errorf("retrieving %s: `model` was nil", *id) | |
} | |
payload := resp.Model | |
if metadata.ResourceData.HasChange("tags") { | |
payload.Tags = pointer.To(model.Tags) | |
} | |
if err := client.CreateOrUpdateThenPoll(ctx, *id, *payload, fleets.DefaultCreateOrUpdateOperationOptions()); err != nil { | |
return fmt.Errorf("updating %s: %+v", *id, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sry... did a rebase and made the labels bit weird....
And can't see where I can update those, maybe I don't have permission, can you help check if you can fix the labels?
e0873e9
to
e2d2c21
Compare
e2d2c21
to
34d8150
Compare
Update kubernetes_fleet_manager_resource.go Update kubernetes_fleet_manager_resource.go Update kubernetes_fleet_manager_resource_test.go Update kubernetes_fleet_manager.html.markdown Update kubernetes_fleet_manager.html.markdown update update
e026d97
to
da5a458
Compare
Community Note
Description
azurerm_kubernetes_fleet_manager
- Add support forhub_profile
propertyThe current api version 2024-04-01 now supports the hub_profile property.
This resource is originally auto generated by pandora, but the generated code for api version 2024-04-01 does not work and needs to be implemented manually.
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_kubernetes_fleet_manager
- Add support forhub_profile
propertyThis is a (please select all that apply):
Related Issue(s)
Note
If this PR changes meaningfully during the course of review please update the title and description as required.