gh-628 fix deletion of dnsrecord and certificates on deletion of gateway target for dnspolicy and tlspolicy

[laurafitzgerald]

Addressed #628

Verification

DNSRecord

Follow the steps described in the issue, the dnsrecord should be deleted.

Health Check Probes

kubectl create secret generic probe-headers --from-literal=test=dGVzdAo= -n multi-cluster-gateways

Follow the steps describe in the issue with the following adjustments.
Add the following block to your dnspolicy

healthCheck:
      additionalHeadersRef:
        name: probe-headers
      allowInsecureCertificates: true
      endpoint: /
      expectedResponses:
      - 200
      - 201
      - 301
      failureThreshold: 1
      port: 443
      protocol: https

there should be one dnshealthcheck probe present in the cluster now
e.g.

kc get dnshealthcheckprobes -A        

NAMESPACE                NAME                        HEALTHY   LAST CHECKED
multi-cluster-gateways   172.31.200.0-prod-web-api   true      3s

Delete the dnspolicy, the healthcheck probe should be deleted.

Certificates

• create gateway
• label gateway
• create tlspolicy
• verify certificates are created
• delete gateway
• verify certificates are deleted.

[mikenairn]

Looks good, just a couple of small changes that i think we need to make sure we aren't deleting the wrong resources when processing the gateways that have an invalid policy ref.

[laurafitzgerald]

The e2e test failure here is intermittent and appears to be not related to the change.
The failure happens on error [debug] Cert error: 'wildcard hostname not found in the certificate via get request' which is in relation to tlspolicy and secret creation. The first two failures were on the AWS suite and the third is on GCP suite. Locally this test is passing for me on AWS backed cluster.

EDIT: e2e test are passing now. Seems it was intermittent/unrelated.

Aside from this feedback has been addressed and verification steps are updated.

I will keep investigating the failure but it seems intermittent and unrelated.

cc @mikenairn

[mikenairn]

Any reason you have removed the deleteUnexpectedGatewayCertificates method here, but chose to keep deleteUnexpectedGatewayProbes in the dnspolicy_healthchecks reconcile?

[laurafitzgerald]

My thinking at the time was to bring it in line with the DNSRecords reconciliation, but looking at it now it makes sense to keep it. I'm bringing the three more in line with each other so I'll return the unexpectedcertificates to match the unexpectedhealthcheckprobes. Doesn't seem a need for it in the dnsrecords reconciliation though so I wont add it there. Commit coming soon.

[mikenairn]

@laurafitzgerald Looks good to me. Left a couple of comments, will let you reply to those before sticking the lgtm on it.

[mikenairn]

Personally i kind of preferred the pattern this (and DNSRecord) was using of having a reconcileGateway<Resource> method that deals with reconciling resources for a given Gateway instead of having it broken up inside the main reconcile<Resource> method.

• reconcile<Resource> - Reconcile resources(create/update/delete) for the current policy
• reconcileGateway<Resource> - Reconcile resources(create/update/delete) for the current policy and gateway
• delete<Resource> - Delete resources for the current policy
• deleteGateway<Resource> - Delete resources for the current policy and gateway

I'd say if we are trying to make these controllers consistent (looks like you have copied the pattern from health checks) we should actually update the DNSRecord one to deal with unexpected gateway resources in the same way as the other two.

Since none of that is in the scope of this bug fix i'd say we can come back to this later.

[mikenairn]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: laurafitzgerald, mikenairn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mikenairn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment