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

Code generate ResourceGroup #2748

Merged
merged 3 commits into from
Mar 9, 2023
Merged

Conversation

matthchr
Copy link
Member

Removes the handcrafted ResourceGroup in favor of a code generated one. The two are the same in structure except for Status.ProvisioningState, which is now Status.Properties.ProvisioningState (matching ARM).

Special notes for your reviewer:
This change is breaking for:

  1. Consumers of the status provisioningState
  2. Consumers of the API types directly (which we currently don't give a go mod guarantee for anyway)

If applicable:

  • this PR contains documentation
  • this PR contains tests

@matthchr
Copy link
Member Author

Note that I had to edit the recordings but only because the generated code has the same properties in a slightly different order which impacts JSON serialization order. In practice this isn't an issue.

I specifically tested old RG to new RG in kind manually by installing the old shape, then installing the new CRDs and operator and using the new shape -- exisitng RGs get reconciled just fine. This is as expected because the JSON shape in etcd hasn't changed even though the storage type has.

@matthchr matthchr force-pushed the feature/generate-rg branch 2 times, most recently from 613ceef to 648a2c6 Compare February 24, 2023 21:30
@matthchr matthchr added the breaking A change which likely be breaking label Feb 25, 2023
@matthchr
Copy link
Member Author

matthchr commented Mar 9, 2023

/ok-to-test sha=9a72b1f

@codecov-commenter
Copy link

Codecov Report

Merging #2748 (9a72b1f) into main (cde9fa2) will decrease coverage by 0.01%.
The diff coverage is 50.62%.

@@            Coverage Diff             @@
##             main    #2748      +/-   ##
==========================================
- Coverage   56.01%   56.01%   -0.01%     
==========================================
  Files         973      978       +5     
  Lines      349026   348295     -731     
==========================================
- Hits       195520   195091     -429     
+ Misses     122687   122373     -314     
- Partials    30819    30831      +12     
Impacted Files Coverage Δ
v2/internal/controllers/controller_resources.go 78.44% <ø> (-0.87%) ⬇️
.../generator/internal/codegen/pipeline/load_types.go 7.31% <0.00%> (-0.04%) ⬇️
.../internal/functions/locatable_resource_function.go 0.00% <0.00%> (ø)
...nerator/internal/jsonast/swagger_type_extractor.go 17.86% <0.00%> (-0.10%) ⬇️
...a1api20200601/resource_group_spec_arm_types_gen.go 33.33% <33.33%> (ø)
...1beta20200601/resource_group_spec_arm_types_gen.go 33.33% <33.33%> (ø)
.../v1beta20200601storage/resource_group_types_gen.go 44.82% <44.82%> (ø)
...pha1api20200601storage/resource_group_types_gen.go 46.04% <46.04%> (ø)
...es/v1alpha1api20200601/resource_group_types_gen.go 50.48% <50.48%> (ø)
...sources/v1beta20200601/resource_group_types_gen.go 57.42% <57.42%> (ø)
... and 82 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@matthchr
Copy link
Member Author

matthchr commented Mar 9, 2023

/ok-to-test sha=9a72b1f

@matthchr matthchr merged commit c487d18 into Azure:main Mar 9, 2023
@matthchr matthchr deleted the feature/generate-rg branch March 9, 2023 17:24
@matthchr matthchr self-assigned this Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A change which likely be breaking
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants