Skip to content

Conversation

@ironcladlou
Copy link

@ironcladlou ironcladlou commented May 28, 2019

Refactor DNS handling to consolidate how the desired set of DNS records is
computed. This ensures that the same function can be used for both setting up
and tearing down DNS.

Add some unit testing around the desired DNS function.

TODO

  • e2e test creates
  • unit tests
  • e2e test deletes

Refactor DNS handling to consolidate how the desired set of DNS records is
computed. This ensures that the same function can be used for both setting up
and tearing down DNS.

Add some unit testing around the desired DNS function.
@ironcladlou
Copy link
Author

DNS successfully syncs with this branch — will fix up the unit test tomorrow.

@ironcladlou
Copy link
Author

Latest commit adds unit tests, next up I have to do some e2e testing around delete handling and then I think this is ready.

@ironcladlou ironcladlou changed the title WIP: Refactor desired DNS functions Refactor desired DNS functions May 29, 2019
@ironcladlou
Copy link
Author

Okay, I've tested add and delete scenarios with multiple ingress controllers. There's some work to do in the client to clean up how we check for existing DNS records during delete, but we can do that in a followup.

_, err := c.recordSets.Delete(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A, "")
_, err := c.recordSets.Get(ctx, zone.ResourceGroup, zone.Name, arec.Name, dns.A)
if err != nil {
// TODO: How do we interpret this as a notfound error?
Copy link
Owner

Choose a reason for hiding this comment

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

snippet :

package errors

import (
	"github.com/Azure/go-autorest/autorest"
)

func IsNotFound(err error) bool {
	if derr, ok := err.(autorest.DetailedError); ok && derr.StatusCode == 404 {
		return true
	}
	return false
}

@serbrech serbrech merged commit b69ec46 into serbrech:azure-provider May 29, 2019
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.

4 participants