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

ProviderConfig can be deleted before ProviderConfigUsage is getting cleaned #85

Closed
ytsarev opened this issue Apr 11, 2022 · 4 comments · Fixed by #86
Closed

ProviderConfig can be deleted before ProviderConfigUsage is getting cleaned #85

ytsarev opened this issue Apr 11, 2022 · 4 comments · Fixed by #86
Assignees
Labels
bug Something isn't working

Comments

@ytsarev
Copy link

ytsarev commented Apr 11, 2022

What happened?

The ProviderConfig of postgresql.sql.crossplane.io is getting deleted before associated ProviderConfigUsage is getting removed.

It leads to a Database resource that is using this ProviderConfig being stuck in an orphaned state.

How can we reproduce it?

Create ProviderConfig like

apiVersion: postgresql.sql.crossplane.io/v1alpha1
kind: ProviderConfig
metadata:
  name: smalpe-db-bd848
spec:
  credentials:
    connectionSecretRef:
      name: azpgserver-conn-str
      namespace: test
    source: PostgreSQLConnectionSecret
  defaultDatabase: postgres

Create Database referencing this ProviderConfig

apiVersion: postgresql.sql.crossplane.io/v1alpha1
kind: Database
metadata:
  name: smalpe-db-bd848
spec:
  forProvider:
    allowConnections: true
    connectionLimit: -1
    encoding: UTF8
    isTemplate: false
    lcCType: English_United States.1252
    lcCollate: English_United States.1252
    owner: masteradmin
    tablespace: pg_default
  providerConfigRef:
    name: smalpe-db-bd848

Observe that associated ProviderConfigUsage was created

k get providerconfigusages.postgresql.sql.crossplane.io
NAME                                   AGE     CONFIG-NAME       RESOURCE-KIND   RESOURCE-NAME
1366b088-bd57-4286-864a-de44ef90f599   26m     smalpe-db-bd848   Database        smalpe-db-bd848

Delete ProviderConfig

k delete providerconfigs.postgresql.sql.crossplane.io smalpe-db-bd848
providerconfig.postgresql.sql.crossplane.io "smalpe-db-bd848" deleted

Observer that ProviderConfigUsage is still around

k get providerconfigusages.postgresql.sql.crossplane.io
NAME                                   AGE     CONFIG-NAME       RESOURCE-KIND   RESOURCE-NAME
1366b088-bd57-4286-864a-de44ef90f599   27m     smalpe-db-bd848   Database        smalpe-db-bd848

Try to delete the Database resource

k delete database.postgresql.sql.crossplane.io/smalpe-db-bd848

Observer that Database deletion is stuck with

k describe database.postgresql.sql.crossplane.io/smalpe-db-bd848
....
  Warning  CannotConnectToProvider      12s (x12 over 33s)  managed/database.postgresql.sql.crossplane.io  cannot get ProviderConfig: ProviderConfig.postgresql.sql.crossplane.io "smalpe-db-bd848" not found

This issue heavily affects the situation when ProviderConfig and Database are a part of the same Composition.

When the associated Claim is deleted the ProviderConfig is getting deleted first and the Database deletion is stuck.

What environment did it happen in?

Crossplane version: 1.6.3-up.1

@ytsarev ytsarev added the bug Something isn't working label Apr 11, 2022
@ytsarev ytsarev changed the title providerconfigs.postgresql.sql.crossplane.io is getting deleted before providerconfigusages.postgresql.sql.crossplane.io is getting cleaned ProviderConfig can be deleted before ProviderConfigUsage is getting cleaned Apr 11, 2022
@ytsarev
Copy link
Author

ytsarev commented Apr 11, 2022

Providing sample Composition and XRD in addition to the reproducer above.

Composition

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  labels:
    kind: azurepgdatabase
    provider: azure
  name: azurepgdatabase
spec:
  compositeTypeRef:
    apiVersion: reproducer.upbound.io/v1alpha1
    kind: AzurePgDatabaseComposite
  patchSets:
  - name: Metadata
    patches:
    - fromFieldPath: metadata.labels
      type: FromCompositeFieldPath
    - fromFieldPath: metadata.annotations
      type: FromCompositeFieldPath
  resources:
  - base:
      apiVersion: postgresql.sql.crossplane.io/v1alpha1
      kind: ProviderConfig
      spec:
        credentials:
          source: PostgreSQLConnectionSecret
    patches:
    - patchSetName: Metadata
      type: PatchSet
    - fromFieldPath: metadata.name
      type: FromCompositeFieldPath
    - fromFieldPath: spec.parameters.connection_secret_name
      toFieldPath: spec.credentials.connectionSecretRef.name
      type: FromCompositeFieldPath
    - fromFieldPath: metadata.labels["crossplane.io/claim-namespace"]
      toFieldPath: spec.credentials.connectionSecretRef.namespace
      type: FromCompositeFieldPath
    readinessChecks:
    - fieldPath: kind
      matchString: ProviderConfig
      type: MatchString
  - base:
      apiVersion: postgresql.sql.crossplane.io/v1alpha1
      kind: Database
      spec:
        forProvider:
          allowConnections: true
    connectionDetails:
    - fromFieldPath: metadata.annotations["crossplane.io/external-name"]
      name: dbname
      type: FromFieldPath
    patches:
    - patchSetName: Metadata
      type: PatchSet
    - fromFieldPath: metadata.name
      toFieldPath: spec.providerConfigRef.name
      type: FromCompositeFieldPath
    - fromFieldPath: metadata.name
      toFieldPath: metadata.name
      type: FromCompositeFieldPath
    - fromFieldPath: metadata.name
      toFieldPath: status.database_name
      type: ToCompositeFieldPath
  writeConnectionSecretsToNamespace: crossplane-system

XRD

apiVersion: apiextensions.crossplane.io/v1
kind: CompositeResourceDefinition
metadata:
  name: azurepgdatabasecomposites.reproducer.upbound.io
spec:
  claimNames:
    kind: AzurePgDatabase
    plural: azurepgdatabases
  connectionSecretKeys:
  - dbname
  defaultCompositionRef:
    name: azurepgdatabase
  group: reproducer.upbound.io
  names:
    kind: AzurePgDatabaseComposite
    plural: azurepgdatabasecomposites
  versions:
  - name: v1alpha1
    referenceable: true
    schema:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              parameters:
                properties:
                  connection_secret_name:
                    description: Connection secret name used to connect to the PostgreSQL
                      server.
                    type: string
                required:
                - connection_secret_name
                type: object
            required:
            - parameters
            type: object
          status:
            properties:
              database_name:
                description: Name of the created database.
                type: string
            type: object
        type: object
    served: true

Claim

kind: AzurePgDatabase
apiVersion: reproducer.upbound.io/v1alpha1
metadata:
  name: smalpe-db
  namespace: test
spec:
  compositionSelector:
    matchLabels:
      kind: azurepgdatabase
      provider: azure
  parameters:
    connection_secret_name: azpgserver-conn-str
  writeConnectionSecretToRef:
    name: dbconn-test

@ulucinar
Copy link
Collaborator

Thank you @ytsarev for reporting this issue. It looks like we have an issue in the ProviderConfig, where the spec.sslMode is required and enumerated, whereas we do not treat it so. Could you please try the following ProviderConfig:

apiVersion: postgresql.sql.crossplane.io/v1alpha1
kind: ProviderConfig
metadata:
  name: smalpe-db-bd848
spec:
  credentials:
    connectionSecretRef:
      name: azpgserver-conn-str
      namespace: test
    source: PostgreSQLConnectionSecret
  defaultDatabase: postgres
  sslMode: verify-full

What happens is when you provision the ProviderConfig.postgresql without the sslMode, Crossplane runtime cannot add the finalizer that's supposed to prevent deletion of the provider config if there are any associated providerconfig usages with it.

@ytsarev
Copy link
Author

ytsarev commented Apr 12, 2022

Thank you so much @ulucinar, it worked! Do we plan to provide long-term fix?

@ulucinar ulucinar self-assigned this Apr 12, 2022
@ulucinar
Copy link
Collaborator

ulucinar commented Apr 12, 2022

Hi @ytsarev,
Sure thing. I've opened #86 as the fix for this issue. It looks like we have erroneously marked the field as optional although it's not treated as such and when Crossplane tries to add the finalizer to protect the ProviderConfig, it fails. With the proposed fix, this should no longer be the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants