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

Configurable maxConcurrentReconciles and pollInterval with sensible defaults #73

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

turkenh
Copy link
Contributor

@turkenh turkenh commented Nov 10, 2022

Description of your changes

This PR makes maxConcurrentReconciles and pollInterval configurable with the proposed defaults experimented here.
It follows the same approach we used in the community providers to use input arguments as internal configurations.

I have:

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

How has this code been tested

See crossplane/upjet#116 (comment)

…aults

Signed-off-by: Hasan Turken <turkenh@gmail.com>
@turkenh
Copy link
Contributor Author

turkenh commented Nov 10, 2022

Weirdly make check-diff fails on my local but passes in CI with changes in fields descriptions as below:

$ make check-diff
 
22:43:02 [ .. ] generating provider schema for hashicorp/azurerm 3.8.0
22:43:04 [ OK ] generating provider schema for hashicorp/azurerm 3.8.0
22:43:04 [ .. ] verify go modules dependencies have expected content
...

 M package/crds/sql.azure.upbound.io_mssqlmanagedinstances.yaml
 M package/crds/sql.azure.upbound.io_mssqlservers.yaml
 M package/crds/storage.azure.upbound.io_managementpolicies.yaml
 M package/crds/storage.azure.upbound.io_shares.yaml
 M package/crds/storage.azure.upbound.io_tables.yaml
 M package/crds/streamanalytics.azure.upbound.io_outputsynapses.yaml
22:40:36 [FAIL]
make: *** [check-diff] Error 1
$ git diff

--- a/apis/authorization/v1beta1/zz_policydefinition_types.go
+++ b/apis/authorization/v1beta1/zz_policydefinition_types.go
@@ -39,8 +39,7 @@ type PolicyDefinitionParameters struct {
        // +kubebuilder:validation:Optional
        Metadata *string `json:"metadata,omitempty" tf:"metadata,omitempty"`

-       // The policy mode that allows you to specify which resource
-       // types will be evaluated. Possible values are All, Indexed, Microsoft.ContainerService.Data, Microsoft.CustomerLockbox.Data, Microsoft.DataCatalog.Data, Microsoft.KeyVault.Data, Microsoft.Kubernetes.Data, Microsoft.MachineLearningServices.Data, Microsoft.Network.Data and Microsoft.Synapse.Data.
+       // The policy resource manager mode that allows you to specify which resource types will be evaluated. Possible values are All or Indexed.
        // +kubebuilder:validation:Required
        Mode *string `json:"mode" tf:"mode,omitempty"`

diff --git a/apis/compute/v1beta1/zz_linuxvirtualmachine_types.go b/apis/compute/v1beta1/zz_linuxvirtualmachine_types.go
index 03cc4f9..d6fbaaf 100755
--- a/apis/compute/v1beta1/zz_linuxvirtualmachine_types.go
+++ b/apis/compute/v1beta1/zz_linuxvirtualmachine_types.go
@@ -216,7 +216,7 @@ type LinuxVirtualMachineParameters struct {
        // +kubebuilder:validation:Optional
        EncryptionAtHostEnabled *bool `json:"encryptionAtHostEnabled,omitempty" tf:"encryption_at_host_enabled,omitempty"`

-       // Specifies what should happen when the Virtual Machine is evicted for price reasons when using a Spot instance. At this time the only supported value is Deallocate. Changing this forces a new resource to be created.
+       // Specifies what should happen when the Virtual Machine is evicted for price reasons when using a Spot instance. Possible values are Deallocate and Delete. Changing this forces a new resource to be created.
        // +kubebuilder:validation:Optional
        EvictionPolicy *string `json:"evictionPolicy,omitempty" tf:"eviction_policy,omitempty"`

...

@ulucinar do you have any idea on what could cause such a difference?

@turkenh
Copy link
Contributor Author

turkenh commented Nov 10, 2022

Just committed the resulting schema: cfe6bdc

I can drop or keep it based accordingly.

@turkenh
Copy link
Contributor Author

turkenh commented Nov 14, 2022

@ulucinar I guess something was wrong with my local state previously. Retried after a make clean and it worked fine this time with no difference. Dropped that commit back.

Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

We have reviewed the changes proposed here as part of this PR. Thank you @turkenh.

@turkenh turkenh merged commit a27c2b4 into crossplane-contrib:main Nov 14, 2022
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.

2 participants