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.

Sign up for GitHub

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

Jump to bottom

AWSSD: Cleanup empty Services #2510

Merged
merged 3 commits into from
Feb 12, 2022
Merged

AWSSD: Cleanup empty Services #2510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a0601c
AWSSD Remove Empty Service
voro015 Nov 15, 2021
8c951e8
Merge branch 'master' into aws-sd-delete-empty-services
voro015 Jan 9, 2022
8d6b3d3
Removed old comments
voro015 Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func main() {
log.Infof("Registry \"%s\" cannot be used with AWS Cloud Map. Switching to \"aws-sd\".", cfg.Registry)
cfg.Registry = "aws-sd"
}
p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.AWSAssumeRole, cfg.DryRun)
p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.AWSAssumeRole, cfg.DryRun, cfg.AWSSDServiceCleanup, cfg.TXTOwnerID)
case "azure-dns", "azure":
p, err = azure.NewAzureProvider(cfg.AzureConfigFile, domainFilter, zoneNameFilter, zoneIDFilter, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun)
case "azure-private-dns":
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/externaldns/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ type Config struct {
AWSAPIRetries int
AWSPreferCNAME bool
AWSZoneCacheDuration time.Duration
AWSSDServiceCleanup bool
AzureConfigFile string
AzureResourceGroup string
AzureSubscriptionID string
Expand Down Expand Up @@ -219,6 +220,7 @@ var defaultConfig = &Config{
AWSAPIRetries: 3,
AWSPreferCNAME: false,
AWSZoneCacheDuration: 0 * time.Second,
AWSSDServiceCleanup: false,
AzureConfigFile: "/etc/kubernetes/azure.json",
AzureResourceGroup: "",
AzureSubscriptionID: "",
Expand Down Expand Up @@ -409,6 +411,7 @@ func (cfg *Config) ParseFlags(args []string) error {
app.Flag("aws-api-retries", "When using the AWS provider, set the maximum number of retries for API calls before giving up.").Default(strconv.Itoa(defaultConfig.AWSAPIRetries)).IntVar(&cfg.AWSAPIRetries)
app.Flag("aws-prefer-cname", "When using the AWS provider, prefer using CNAME instead of ALIAS (default: disabled)").BoolVar(&cfg.AWSPreferCNAME)
app.Flag("aws-zones-cache-duration", "When using the AWS provider, set the zones list cache TTL (0s to disable).").Default(defaultConfig.AWSZoneCacheDuration.String()).DurationVar(&cfg.AWSZoneCacheDuration)
app.Flag("aws-sd-service-cleanup", "When using the AWS CloudMap provider, delete empty Services without endpoints (default: disabled)").BoolVar(&cfg.AWSSDServiceCleanup)
app.Flag("azure-config-file", "When using the Azure provider, specify the Azure configuration file (required when --provider=azure").Default(defaultConfig.AzureConfigFile).StringVar(&cfg.AzureConfigFile)
app.Flag("azure-resource-group", "When using the Azure provider, override the Azure resource group to use (required when --provider=azure-private-dns)").Default(defaultConfig.AzureResourceGroup).StringVar(&cfg.AzureResourceGroup)
app.Flag("azure-subscription-id", "When using the Azure provider, specify the Azure configuration file (required when --provider=azure-private-dns)").Default(defaultConfig.AzureSubscriptionID).StringVar(&cfg.AzureSubscriptionID)
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/externaldns/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ var (
AWSAPIRetries: 3,
AWSPreferCNAME: false,
AWSZoneCacheDuration: 0 * time.Second,
AWSSDServiceCleanup: false,
AzureConfigFile: "/etc/kubernetes/azure.json",
AzureResourceGroup: "",
AzureSubscriptionID: "",
Expand Down Expand Up @@ -153,6 +154,7 @@ var (
AWSAPIRetries: 13,
AWSPreferCNAME: true,
AWSZoneCacheDuration: 10 * time.Second,
AWSSDServiceCleanup: true,
AzureConfigFile: "azure.json",
AzureResourceGroup: "arg",
AzureSubscriptionID: "arg",
Expand Down Expand Up @@ -304,6 +306,7 @@ func TestParseFlags(t *testing.T) {
"--aws-api-retries=13",
"--aws-prefer-cname",
"--aws-zones-cache-duration=10s",
"--aws-sd-service-cleanup",
"--no-aws-evaluate-target-health",
"--policy=upsert-only",
"--registry=noop",
Expand Down Expand Up @@ -404,6 +407,7 @@ func TestParseFlags(t *testing.T) {
"EXTERNAL_DNS_AWS_API_RETRIES": "13",
"EXTERNAL_DNS_AWS_PREFER_CNAME": "true",
"EXTERNAL_DNS_AWS_ZONES_CACHE_DURATION": "10s",
"EXTERNAL_DNS_AWS_SD_SERVICE_CLEANUP": "true",
"EXTERNAL_DNS_POLICY": "upsert-only",
"EXTERNAL_DNS_REGISTRY": "noop",
"EXTERNAL_DNS_TXT_OWNER_ID": "owner-1",
Expand Down
43 changes: 38 additions & 5 deletions provider/awssd/aws_sd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type AWSSDClient interface {
ListServicesPages(input *sd.ListServicesInput, fn func(*sd.ListServicesOutput, bool) bool) error
RegisterInstance(input *sd.RegisterInstanceInput) (*sd.RegisterInstanceOutput, error)
UpdateService(input *sd.UpdateServiceInput) (*sd.UpdateServiceOutput, error)
DeleteService(input *sd.DeleteServiceInput) (*sd.DeleteServiceOutput, error)
}

// AWSSDProvider is an implementation of Provider for AWS Cloud Map.
Expand All @@ -81,10 +82,14 @@ type AWSSDProvider struct {
namespaceFilter endpoint.DomainFilter
// filter namespace by type (private or public)
namespaceTypeFilter *sd.NamespaceFilter
// enables service without instances cleanup
cleanEmptyService bool
// filter services for removal
ownerID string
}

// NewAWSSDProvider initializes a new AWS Cloud Map based Provider.
func NewAWSSDProvider(domainFilter endpoint.DomainFilter, namespaceType string, assumeRole string, dryRun bool) (*AWSSDProvider, error) {
func NewAWSSDProvider(domainFilter endpoint.DomainFilter, namespaceType string, assumeRole string, dryRun, cleanEmptyService bool, ownerID string) (*AWSSDProvider, error) {
config := aws.NewConfig()

config = config.WithHTTPClient(
Expand Down Expand Up @@ -113,9 +118,11 @@ func NewAWSSDProvider(domainFilter endpoint.DomainFilter, namespaceType string,

provider := &AWSSDProvider{
client: sd.New(sess),
dryRun: dryRun,
namespaceFilter: domainFilter,
namespaceTypeFilter: newSdNamespaceFilter(namespaceType),
dryRun: dryRun,
cleanEmptyService: cleanEmptyService,
ownerID: ownerID,
}

return provider, nil
Expand Down Expand Up @@ -162,6 +169,12 @@ func (p *AWSSDProvider) Records(ctx context.Context) (endpoints []*endpoint.Endp
ep := p.instancesToEndpoint(ns, srv, instances)
endpoints = append(endpoints, ep)
}
if len(instances) == 0 {
err = p.DeleteService(srv)
if err != nil {
log.Warnf("Failed to delete service \"%s\", error: %s", aws.StringValue(srv.Name), err)
}
}
}
}

Expand Down Expand Up @@ -285,9 +298,8 @@ func (p *AWSSDProvider) submitCreates(namespaces []*sd.NamespaceSummary, changes
}
// update local list of services
services[*srv.Name] = srv
} else if (ch.RecordTTL.IsConfigured() && *srv.DnsConfig.DnsRecords[0].TTL != int64(ch.RecordTTL)) ||
aws.StringValue(srv.Description) != ch.Labels[endpoint.AWSSDDescriptionLabel] {
// update service when TTL or Description differ
} else if ch.RecordTTL.IsConfigured() && *srv.DnsConfig.DnsRecords[0].TTL != int64(ch.RecordTTL) {
// update service when TTL differ
err = p.UpdateService(srv, ch)
if err != nil {
return err
Expand Down Expand Up @@ -491,6 +503,27 @@ func (p *AWSSDProvider) UpdateService(service *sd.Service, ep *endpoint.Endpoint
return nil
}

// DeleteService deletes empty Service from AWS API if its owner id match
func (p *AWSSDProvider) DeleteService(service *sd.Service) error {
log.Debugf("Check if service \"%s\" owner id match and it can be deleted", *service.Name)
if !p.dryRun && p.cleanEmptyService {
// convert ownerID string to service description format
label := endpoint.NewLabels()
label[endpoint.OwnerLabelKey] = p.ownerID
label[endpoint.AWSSDDescriptionLabel] = label.Serialize(false)

if aws.StringValue(service.Description) == label[endpoint.AWSSDDescriptionLabel] {
log.Infof("Deleting service \"%s\"", *service.Name)
_, err := p.client.DeleteService(&sd.DeleteServiceInput{
Id: aws.String(*service.Id),
})
return err
}
log.Debugf("Skipping service removal %s because owner id does not match, found: \"%s\", required: \"%s\"", aws.StringValue(service.Name), aws.StringValue(service.Description), label[endpoint.AWSSDDescriptionLabel])
}
return nil
}

// RegisterInstance creates a new instance in given service.
func (p *AWSSDProvider) RegisterInstance(service *sd.Service, ep *endpoint.Endpoint) error {
for _, target := range ep.Targets {
Expand Down
87 changes: 76 additions & 11 deletions provider/awssd/aws_sd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,27 @@ func (s *AWSSDClientStub) UpdateService(input *sd.UpdateServiceInput) (*sd.Updat
return &sd.UpdateServiceOutput{}, nil
}

func newTestAWSSDProvider(api AWSSDClient, domainFilter endpoint.DomainFilter, namespaceTypeFilter string) *AWSSDProvider {
func (s *AWSSDClientStub) DeleteService(input *sd.DeleteServiceInput) (*sd.DeleteServiceOutput, error) {
out, err := s.GetService(&sd.GetServiceInput{Id: input.Id})
if err != nil {
return nil, err
}

service := out.Service
namespace := s.services[*service.NamespaceId]
delete(namespace, *input.Id)

return &sd.DeleteServiceOutput{}, nil
}

func newTestAWSSDProvider(api AWSSDClient, domainFilter endpoint.DomainFilter, namespaceTypeFilter, ownerID string) *AWSSDProvider {
return &AWSSDProvider{
client: api,
dryRun: false,
namespaceFilter: domainFilter,
namespaceTypeFilter: newSdNamespaceFilter(namespaceTypeFilter),
dryRun: false,
cleanEmptyService: true,
ownerID: ownerID,
}
}

Expand Down Expand Up @@ -288,7 +303,7 @@ func TestAWSSDProvider_Records(t *testing.T) {
instances: instances,
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

endpoints, _ := provider.Records(context.Background())

Expand Down Expand Up @@ -316,7 +331,7 @@ func TestAWSSDProvider_ApplyChanges(t *testing.T) {
{DNSName: "service3.private.com", Targets: endpoint.Targets{"cname.target.com"}, RecordType: endpoint.RecordTypeCNAME, RecordTTL: 100},
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

ctx := context.Background()

Expand Down Expand Up @@ -376,7 +391,7 @@ func TestAWSSDProvider_ListNamespaces(t *testing.T) {
{"domain filter", endpoint.NewDomainFilter([]string{"public.com"}), "", []*sd.NamespaceSummary{namespaceToNamespaceSummary(namespaces["public"])}},
{"non-existing domain", endpoint.NewDomainFilter([]string{"xxx.com"}), "", []*sd.NamespaceSummary{}},
} {
provider := newTestAWSSDProvider(api, tc.domainFilter, tc.namespaceTypeFilter)
provider := newTestAWSSDProvider(api, tc.domainFilter, tc.namespaceTypeFilter, "")

result, err := provider.ListNamespaces()
require.NoError(t, err)
Expand Down Expand Up @@ -439,7 +454,7 @@ func TestAWSSDProvider_ListServicesByNamespace(t *testing.T) {
}{
{map[string]*sd.Service{"service1": services["private"]["srv1"], "service2": services["private"]["srv2"]}},
} {
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

result, err := provider.ListServicesByNamespaceID(namespaces["private"].Id)
require.NoError(t, err)
Expand Down Expand Up @@ -495,7 +510,7 @@ func TestAWSSDProvider_ListInstancesByService(t *testing.T) {
instances: instances,
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

result, err := provider.ListInstancesByServiceID(services["private"]["srv1"].Id)
require.NoError(t, err)
Expand Down Expand Up @@ -532,7 +547,7 @@ func TestAWSSDProvider_CreateService(t *testing.T) {

expectedServices := make(map[string]*sd.Service)

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

// A type
provider.CreateService(aws.String("private"), aws.String("A-srv"), &endpoint.Endpoint{
Expand Down Expand Up @@ -636,7 +651,7 @@ func TestAWSSDProvider_UpdateService(t *testing.T) {
services: services,
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

// update service with different TTL
provider.UpdateService(services["private"]["srv1"], &endpoint.Endpoint{
Expand All @@ -647,6 +662,56 @@ func TestAWSSDProvider_UpdateService(t *testing.T) {
assert.Equal(t, int64(100), *api.services["private"]["srv1"].DnsConfig.DnsRecords[0].TTL)
}

func TestAWSSDProvider_DeleteService(t *testing.T) {
namespaces := map[string]*sd.Namespace{
"private": {
Id: aws.String("private"),
Name: aws.String("private.com"),
Type: aws.String(sd.NamespaceTypeDnsPrivate),
},
}

services := map[string]map[string]*sd.Service{
"private": {
"srv1": {
Id: aws.String("srv1"),
Description: aws.String("heritage=external-dns,external-dns/owner=owner-id"),
Name: aws.String("service1"),
NamespaceId: aws.String("private"),
},
"srv2": {
Id: aws.String("srv2"),
Description: aws.String("heritage=external-dns,external-dns/owner=owner-id"),
Name: aws.String("service2"),
NamespaceId: aws.String("private"),
},
},
}

api := &AWSSDClientStub{
namespaces: namespaces,
services: services,
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "owner-id")

// delete fist service
err := provider.DeleteService(services["private"]["srv1"])
assert.NoError(t, err)
assert.Len(t, api.services["private"], 1)

expectedServices := map[string]*sd.Service{
"srv2": {
Id: aws.String("srv2"),
Description: aws.String("heritage=external-dns,external-dns/owner=owner-id"),
Name: aws.String("service2"),
NamespaceId: aws.String("private"),
},
}

assert.Equal(t, expectedServices, api.services["private"])
}

func TestAWSSDProvider_RegisterInstance(t *testing.T) {
namespaces := map[string]*sd.Namespace{
"private": {
Expand Down Expand Up @@ -703,7 +768,7 @@ func TestAWSSDProvider_RegisterInstance(t *testing.T) {
instances: make(map[string]map[string]*sd.Instance),
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

expectedInstances := make(map[string]*sd.Instance)

Expand Down Expand Up @@ -820,7 +885,7 @@ func TestAWSSDProvider_DeregisterInstance(t *testing.T) {
instances: instances,
}

provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "")
provider := newTestAWSSDProvider(api, endpoint.NewDomainFilter([]string{}), "", "")

provider.DeregisterInstance(services["private"]["srv1"], endpoint.NewEndpoint("srv1.private.com.", endpoint.RecordTypeA, "1.2.3.4"))

Expand Down