-
Notifications
You must be signed in to change notification settings - Fork 33
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
tests: Add dns record deletion tests (orphan records) #942
tests: Add dns record deletion tests (orphan records) #942
Conversation
//err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) | ||
//g.Expect(err).NotTo(HaveOccurred()) | ||
//g.Expect(gateway.Annotations).ToNot(HaveKey(v1alpha1.DNSPolicyDirectReferenceAnnotationName)) | ||
//g.Expect(gateway.Annotations).ToNot(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these back references will be removed as part of the SOTW so we don't really need to worry about fixing these. More just here for me to understand what should be happening currently.
), | ||
) | ||
//ToDo (mnairn) Theres a bug here, this never gets reset when all records are removed due to target removal | ||
//g.Expect(dnsPolicy.Status.TotalRecords).To(Equal(int32(0))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issues with the record count will be resolved by the SOTW status reconciler updates.
618721d
to
aee0a40
Compare
{ | ||
Type: ptr.To(gatewayapiv1.IPAddressType), | ||
//Using the same address can cause a panic in the inmemory provider see https://github.com/Kuadrant/dns-operator/issues/272 | ||
Value: "172.1.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KevFan I haven't reviewed the the TLS tests, but we should probably make sure we have appropriate test coverage there also. |
Adds/Updates tests to better cover dnsrecord deletion scenarios. Signed-off-by: Michael Nairn <[email protected]>
aee0a40
to
9ccf0b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Adds/Updates tests to better cover dnsrecord deletion scenarios.
Requires: #941