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.

Sign up for GitHub

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

Jump to bottom

Use Terraform Plugin SDK to Reconcile MRs #592

Merged
merged 35 commits into from
Jan 3, 2024
Merged

Use Terraform Plugin SDK to Reconcile MRs #592

merged 35 commits into from
Jan 3, 2024

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Nov 15, 2023 •
edited
Loading

Description of your changes

This PR employs the controller.noForkExternal & controller.noForkAsyncExternal external clients from upjet to reconcile MRs without forking any Terraform CLI or Terraform provider processes.

Some more details on these clients can be found here.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

@ulucinar ulucinar marked this pull request as draft November 15, 2023 15:43
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/azure/resourcegroup.yaml"

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/azure/resourcegroup.yaml"

@ulucinar ulucinar mentioned this pull request Nov 15, 2023
Merged
1 task
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/azure/resourcegroup.yaml"

@ulucinar ulucinar changed the title Switch to the no-fork architecture for the azurerm resources Use Terraform Plugin SDK to Reconcile MRs Nov 15, 2023
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/azure/resourcegroup.yaml"

@ulucinar
Copy link
Collaborator Author

ulucinar commented Nov 15, 2023 •
edited
Loading

A test with (an earlier version of) upbound/platform-ref-azure has succeeded here:
https://github.com/upbound/platform-ref-azure/actions/runs/6880699930
with the packages index.docker.io/ulucinar/provider-azure-{containerservice,dbformariadb,dbforpostgresql,network}:v0.44.0-55db9b8de10bedf853c1fb00259a5d62bb2f52e9

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/azure/resourcegroup.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/network/subnet.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/containerservice/kubernetescluster.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/network/subnet.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/appactiveslot.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/apphybridconnection.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/functionappactiveslot.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/appactiveslot.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/sourcecontroltoken.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/linuxwebappslot.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/appserviceplan.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/managedidentity/federatedidentitycredential.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/containerservice/kubernetesfleetmanager.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/logz/subaccounttagrule.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/azure/resourcegroup.yaml"

1 similar comment
@sergenyalcin
Copy link
Collaborator

/test-examples="examples/azure/resourcegroup.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/logz/subaccounttagrule.yaml"

erhancagirici and others added 20 commits January 2, 2024 18:39
@erhancagirici @ulucinar
add dependencies for BackupPolicyBlobStorage.dataprotection.azure exa…
984d1ff 
…mple

Signed-off-by: Erhan Cagirici <erhan@upbound.io>
@sergenyalcin @ulucinar
Update upjet dependency to the ulucinar/b674f3dd0c9f9dc238b2bbd5518fa…
c50e201 
…de662d12077 commit to consume the caching ID fix

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Update upjet dependency to 9543be90ce6b1001213a5bb3429bb6947a131fd3 c…
d068fc5 
…ommit

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Fix authorization.ResourceGroupPolicyAssignment example manifest
3f32767 
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@erhancagirici @ulucinar
remove explicit sync mode configuration from resources
306f5b8 
Historically, some resources were explicitly marked to use sync mode due to gain performance, however this is not needed anymore with the no-fork architecture.
Async mode is safer especially for resources with long creation time
Following resources are set back to the default async configuration:
- azurerm_api_management
- azurerm_subscription
- azurerm_resource_provider_registration
- azurerm_management_group
- azurerm_mariadb_configuration
- azurerm_ip_group
- azurerm_network_interface
- azurerm_network_watcher
- azurerm_network_watcher_flow_log
- azurerm_network_connection_monitor
- azurerm_network_ddos_protection_plan
- azurerm_network_security_rule
- azurerm_network_profile
- azurerm_private_dns_a_record
- azurerm_private_dns_aaaa_record
- azurerm_private_dns_mx_record
- azurerm_private_dns_ptr_record
- azurerm_private_dns_srv_record
- azurerm_private_dns_txt_record
- azurerm_frontdoor
- azurerm_application_security_group
- azurerm_private_dns_zone
- azurerm_public_ip
- azurerm_public_ip_prefix
- azurerm_network_security_group
- azurerm_virtual_network
- azurerm_postgresql_flexible_server_configuration
- azurerm_postgresql_configuration
- azurerm_mssql_server_transparent_data_encryption
- azurerm_storage_sync

Signed-off-by: Erhan Cagirici <erhan@upbound.io>
@erhancagirici @ulucinar
generate - remove explicit sync mode configuration from resources
07f21f7 
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
@sergenyalcin @ulucinar
Fix external name configuration of storage.BlobInventoryPolicy resource
00fc4ca 
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Use IdentifierFromProvider for storage.BlobInventoryPolicy resource
08aa075 
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
- Fix external name configuration of storage.EncryptionScope and stor…
00918d5 
…age.ManagementPolicy resources.

- Fix efrontdoor-all-in-one.yaml example manifest

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Fix two network group resources
2360255 
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Fix network.ManagerManagementGroupConnection resource by adding Custo…
52ea37b 
…mDiff

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@ulucinar
Bump upjet to commit 343f24ceaee4
0063f85 
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@sergenyalcin @ulucinar
- Update upjet dependency 81e262067d2b9e7099ca6a07b15e8d2ca1a08b5e
43978cc 
- Add custom diffs to three computer resources

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Add custom diff for azurerm_windows_virtual_machine_scale_set
b272492 
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@erhancagirici @ulucinar
fix RedisCache.apimanagement example manifest
ac9afd4 
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
@erhancagirici @ulucinar
fix missing parantheses for DedicatedHost.compute external name config
bf6a28c 
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
@sergenyalcin @ulucinar
Remove jsonencode keyword from resources.ResourceGroupTemplateDeploym…
e6987c0 
…ent example

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin @ulucinar
Fix external name template of authorization.ResourceGroupPolicyAssign…
1a8765c 
…ment

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@erhancagirici @ulucinar
fix external name config for Gateway/apimanagement.azure
b8933f6 
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
@ulucinar
Run make generate
9d67f2f 
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar
Copy link
Collaborator Author

ulucinar commented Jan 2, 2024

/test-examples="examples/azure/resourcegroup.yaml"

@ulucinar
Copy link
Collaborator Author

ulucinar commented Jan 2, 2024 •
edited
Loading

Hi @sergenyalcin,
With the latest commit, we are losing some of the spec.initProvider cross-resource referencer fields in 6 MRs introduced with crossplane/upjet#315. Have the reference target fields been removed from spec.initProvider? Could you please take a look?

@sergenyalcin
Remove the following fields from IdentifierFields slices:
22f6bb8 
- azurerm_api_management_gateway.api_management_id
- azurerm_resource_group_policy_assignment.resource_group_id
- azurerm_container_connected_registry.container_registry_id
- azurerm_container_registry_webhook.registry_name
- azurerm_storage_encryption_scope.storage_account_id
- azurerm_storage_management_policy.storage_account_id

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin
Copy link
Collaborator

With the latest commit, we are losing some of the spec.initProvider cross-resource referencer fields in 6 MRs introduced with crossplane/upjet#315. Have the reference target fields been removed from spec.initProvider? Could you please take a look?

@ulucinar I checked this. We have external name configuration fixes for the six resources in this PR. Because of the external name configuration templated fixes, some fields are marked as IdentifierField. We do not generate IdentifierFields in InitProvider. The reason for the diff is this. I removed these fields from the IdentifierFields maps. This deletion does not affect the runtime/reconciliation. It only affects the generation. So, now, we again have these removed fields.

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/azure/resourcegroup.yaml"

sergenyalcin
sergenyalcin approved these changes Jan 3, 2024
View reviewed changes
Copy link
Collaborator

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @ulucinar, @erhancagirici, @turkenf LGTM!

@ulucinar ulucinar merged commit 8e737b3 into crossplane-contrib:main Jan 3, 2024
10 checks passed
@ulucinar ulucinar deleted the no-fork-v3.57.0 branch January 3, 2024 15:07
@ulucinar ulucinar mentioned this pull request Jan 3, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergenyalcin sergenyalcin sergenyalcin approved these changes

@turkenf turkenf Awaiting requested review from turkenf turkenf is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ulucinar @sergenyalcin @turkenf @erhancagirici @jeanduplessis