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

Add Support for Observe Only Resources #435

Merged
merged 2 commits into from
Apr 18, 2023

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Apr 18, 2023

Description of your changes

This PR adds support for Observe Only resources. It also bumps up to v0.16.1.
Related PR: crossplane-contrib/provider-upjet-aws#672

I have:

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

How has this code been tested

Provisioned and orphaned a network.VirtualNetwork resource. Then I tried importing it while running the provider with the newly introduced alpha feature disabled. As expected, I observed the managementPolicy is set to a non-default value but the feature is not enabled Synced == false status condition. Then, I enabled the new alpha feature by passing --enable-management-policies command-line option to the provider and successfully imported the managed resource using the following manifest:

apiVersion: network.azure.upbound.io/v1beta1
kind: VirtualNetwork
metadata:
  annotations:
    crossplane.io/external-name: example
  name: example-import
spec:
  managementPolicy: ObserveOnly
  forProvider:
    resourceGroupName: example-virtualnetwork

The full manifest of the imported resource (spec.managementPolicy: ObserveOnly) looks like the following:

apiVersion: network.azure.upbound.io/v1beta1
kind: VirtualNetwork
metadata:
  annotations:
    crossplane.io/external-name: example
  creationTimestamp: "2023-04-18T13:22:00Z"
  generation: 1
  name: example-import
  resourceVersion: "92235"
  uid: 77e3f3ef-1540-4240-adc2-facc300240ca
spec:
  deletionPolicy: Delete
  forProvider:
    resourceGroupName: example-virtualnetwork
  managementPolicy: ObserveOnly
  providerConfigRef:
    name: default
status:
  atProvider:
    addressSpace:
    - 10.0.0.0/16
    bgpCommunity: ""
    edgeZone: ""
    flowTimeoutInMinutes: 0
    guid: 96cbba00-bd68-4375-8d3a-68978ac79099
    id: /subscriptions/.../resourceGroups/example-virtualnetwork/providers/Microsoft.Network/virtualNetworks/example
    location: westeurope
    resourceGroupName: example-virtualnetwork
  conditions:
  - lastTransitionTime: "2023-04-18T13:22:08Z"
    reason: Available
    status: "True"
    type: Ready
  - lastTransitionTime: "2023-04-18T13:22:08Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced

Update and performance regression tests have been done in the context of crossplane-contrib/provider-upjet-aws#672.

@ulucinar ulucinar requested a review from turkenh April 18, 2023 12:13
@ulucinar ulucinar marked this pull request as draft April 18, 2023 12:13
- Bump "up" to v0.16.1

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Copy link
Collaborator Author

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

@ulucinar ulucinar marked this pull request as ready for review April 18, 2023 13:24
@turkenf
Copy link
Collaborator

turkenf commented Apr 18, 2023

We can ignore check-examples job failure. The related issue about this: #433

Copy link
Contributor

@turkenh turkenh left a comment

Choose a reason for hiding this comment

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

Thank you @ulucinar 🙏

}

type ResourceGroupParameters struct {

// The Azure Region where the Resource Group should exist. Changing this forces a new Resource Group to be created.
// +kubebuilder:validation:Required
Location *string `json:"location" tf:"location,omitempty"`
// +kubebuilder:validation:Optional
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, should this be an identifier field 🤔
Could a resource group be observed without location?

Copy link
Collaborator Author

@ulucinar ulucinar Apr 18, 2023

Choose a reason for hiding this comment

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

This resource's external-name configuration is as follows:

config.TemplatedStringAsIdentifier("name", "/subscriptions/{{ .setup.configuration.subscription_id }}/resourceGroups/{{ .external_name }}"),

, which does not refer to the location argument. I think this explains why it's not treated as an identifier argument.

I've also verified that using the following manifest, we can import an existing Azure resource group:

apiVersion: azure.upbound.io/v1beta1
kind: ResourceGroup
metadata:
  annotations:
    crossplane.io/external-name: example-resources
  name: example-import
spec:
  managementPolicy: ObserveOnly
  forProvider: {}

The imported resource's manifest is:

apiVersion: azure.upbound.io/v1beta1
kind: ResourceGroup
metadata:
  annotations:
    crossplane.io/external-name: example-resources
  creationTimestamp: "2023-04-18T17:12:21Z"
  generation: 1
  name: example-import
  resourceVersion: "122209"
  uid: e59544e2-04ca-49d7-92c3-47b73b339ecc
spec:
  deletionPolicy: Delete
  forProvider: {}
  managementPolicy: ObserveOnly
  providerConfigRef:
    name: default
status:
  atProvider:
    id: /subscriptions/.../resourceGroups/example-resources
    location: westeurope
    tags:
      provisioner: crossplane
  conditions:
  - lastTransitionTime: "2023-04-18T17:12:29Z"
    reason: Available
    status: "True"
    type: Ready
  - lastTransitionTime: "2023-04-18T17:12:29Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced

Looks like we are fine.

…PostgreSQL.dataprotection to check-examples.py

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar ulucinar merged commit 17341fd into crossplane-contrib:main Apr 18, 2023
@ulucinar ulucinar deleted the observe-only-support branch April 18, 2023 17:16
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