Skip to content

Commit

Permalink
refactor(dns): don't allocate virtual outbound entries from ZoneIngre…
Browse files Browse the repository at this point in the history 
…ss for own zone (#4439)

* fix(dns): don't allocate virtual outbound entries from ZoneIngress for own zone
* test(pkg/dns): add test that ZoneIngress for local zone is ignored

Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
  • Loading branch information
@michaelbeaumont
michaelbeaumont authored Jun 15, 2022
1 parent 11a644c commit a066dca
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 18 deletions.
33 changes: 19 additions & 14 deletions pkg/dns/vips_allocator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,21 @@ type VIPsAllocator struct {
cidr string
serviceVipEnabled bool
dnsSuffix string
zone string
}

// NewVIPsAllocator creates new object of VIPsAllocator. You can either
// call method CreateOrUpdateVIPConfig manually or start VIPsAllocator as a component.
// In the latter scenario it will call CreateOrUpdateVIPConfig every 'tickInterval'
// for all meshes in the store.
func NewVIPsAllocator(rm manager.ReadOnlyResourceManager, configManager config_manager.ConfigManager, config dns_server.Config) (*VIPsAllocator, error) {
func NewVIPsAllocator(rm manager.ReadOnlyResourceManager, configManager config_manager.ConfigManager, config dns_server.Config, zone string) (*VIPsAllocator, error) {
return &VIPsAllocator{
rm: rm,
persistence: vips.NewPersistence(rm, configManager),
serviceVipEnabled: config.ServiceVipEnabled,
cidr: config.CIDR,
dnsSuffix: config.Domain,
zone: zone,
}, nil
}

Expand All @@ -64,7 +66,7 @@ func (d *VIPsAllocator) CreateOrUpdateVIPConfig(ctx context.Context, mesh string
return err
}

newView, err := BuildVirtualOutboundMeshView(ctx, d.rm, d.serviceVipEnabled, d.dnsSuffix, mesh)
newView, err := d.BuildVirtualOutboundMeshView(ctx, mesh)
if err != nil {
return err
}
Expand All @@ -84,7 +86,7 @@ func (d *VIPsAllocator) createOrUpdateVIPConfigs(ctx context.Context, mesh strin
return err
}

newView, err := BuildVirtualOutboundMeshView(ctx, d.rm, d.serviceVipEnabled, d.dnsSuffix, mesh)
newView, err := d.BuildVirtualOutboundMeshView(ctx, mesh)
if err != nil {
return err
}
Expand Down Expand Up @@ -176,22 +178,22 @@ func addFromMeshGateway(outboundSet *vips.VirtualOutboundMeshView, dnsSuffix, me
}
}

func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResourceManager, serviceVipEnabled bool, dnsSuffix string, mesh string) (*vips.VirtualOutboundMeshView, error) {
func (d *VIPsAllocator) BuildVirtualOutboundMeshView(ctx context.Context, mesh string) (*vips.VirtualOutboundMeshView, error) {
outboundSet := vips.NewEmptyVirtualOutboundView()

virtualOutbounds := core_mesh.VirtualOutboundResourceList{}
if err := rm.List(ctx, &virtualOutbounds, store.ListByMesh(mesh)); err != nil {
if err := d.rm.List(ctx, &virtualOutbounds, store.ListByMesh(mesh)); err != nil {
return nil, err
}
dataplanes := core_mesh.DataplaneResourceList{}
if err := rm.List(ctx, &dataplanes, store.ListByMesh(mesh)); err != nil {
if err := d.rm.List(ctx, &dataplanes, store.ListByMesh(mesh)); err != nil {
return nil, err
}

var errs error
for _, dp := range dataplanes.Items {
for _, inbound := range dp.Spec.GetNetworking().GetInbound() {
if serviceVipEnabled {
if d.serviceVipEnabled {
errs = multierr.Append(errs, addDefault(outboundSet, inbound.GetService(), 0))
}
for _, vob := range Match(virtualOutbounds.Items, inbound.Tags) {
Expand All @@ -201,13 +203,16 @@ func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResour
}

zoneIngresses := core_mesh.ZoneIngressResourceList{}
if err := rm.List(ctx, &zoneIngresses); err != nil {
if err := d.rm.List(ctx, &zoneIngresses); err != nil {
return nil, err
}

for _, zi := range zoneIngresses.Items {
for _, service := range zi.Spec.GetAvailableServices() {
if service.Mesh == mesh && serviceVipEnabled {
if !zi.IsRemoteIngress(d.zone) {
continue
}
if service.Mesh == mesh && d.serviceVipEnabled {
errs = multierr.Append(errs, addDefault(outboundSet, service.GetTags()[mesh_proto.ServiceTag], 0))
}
for _, vob := range Match(virtualOutbounds.Items, service.Tags) {
Expand All @@ -217,12 +222,12 @@ func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResour
}

externalServices := core_mesh.ExternalServiceResourceList{}
if err := rm.List(ctx, &externalServices, store.ListByMesh(mesh)); err != nil {
if err := d.rm.List(ctx, &externalServices, store.ListByMesh(mesh)); err != nil {
return nil, err
}
for _, es := range externalServices.Items {
tags := map[string]string{mesh_proto.ServiceTag: es.Spec.GetService()}
if serviceVipEnabled {
if d.serviceVipEnabled {
errs = multierr.Append(errs, addDefault(outboundSet, es.Spec.GetService(), es.Spec.GetPortUInt32()))
}
errs = multierr.Append(errs, outboundSet.Add(vips.NewHostEntry(es.Spec.GetHost()), vips.OutboundEntry{
Expand All @@ -236,19 +241,19 @@ func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResour
}

meshList := core_mesh.MeshResourceList{}
if err := rm.List(ctx, &meshList); err != nil {
if err := d.rm.List(ctx, &meshList); err != nil {
return nil, err
}

for _, mesh := range meshList.Items {
meshName := mesh.GetMeta().GetName()
gateways := core_mesh.MeshGatewayResourceList{}
if err := rm.List(ctx, &gateways, store.ListByMesh(meshName)); err != nil {
if err := d.rm.List(ctx, &gateways, store.ListByMesh(meshName)); err != nil {
return nil, err
}

for _, gateway := range gateways.Items {
addFromMeshGateway(outboundSet, dnsSuffix, meshName, gateway)
addFromMeshGateway(outboundSet, d.dnsSuffix, meshName, gateway)
}
}

Expand Down
60 changes: 56 additions & 4 deletions pkg/dns/vips_allocator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ var _ = Describe("VIP Allocator", func() {
err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: dp("web")}, store.CreateByKey("dp-3", "mesh-2"))
Expect(err).ToNot(HaveOccurred())

allocator, err = dns.NewVIPsAllocator(rm, cm, testConfig)
allocator, err = dns.NewVIPsAllocator(rm, cm, testConfig, "")
Expect(err).ToNot(HaveOccurred())
})

Expand Down Expand Up @@ -150,7 +150,7 @@ var _ = Describe("VIP Allocator", func() {
It("should return error if failed to update VIP config", func() {
errConfigManager := &errConfigManager{ConfigManager: cm}
ctx := context.Background()
errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, testConfig)
errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, testConfig, "")
Expect(err).ToNot(HaveOccurred())

err = errAllocator.CreateOrUpdateVIPConfig(ctx, "mesh-1", NoModifications)
Expand All @@ -166,7 +166,7 @@ var _ = Describe("VIP Allocator", func() {

It("should try to update all meshes and return combined error", func() {
errConfigManager := &errConfigManager{ConfigManager: cm}
errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, testConfig)
errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, testConfig, "")
Expect(err).ToNot(HaveOccurred())

err = errAllocator.CreateOrUpdateVIPConfigs(context.Background())
Expand Down Expand Up @@ -208,6 +208,7 @@ var _ = Describe("VIP Allocator", func() {

type outboundViewTestCase struct {
givenResources map[model.ResourceKey]model.Resource
whenZone string
whenMesh string
whenSkipServiceVips bool
thenHostnameEntries []vips.HostnameEntry
Expand All @@ -229,8 +230,15 @@ var _ = DescribeTable("outboundView",
Expect(rm.Create(ctx, res, store.CreateBy(k))).ToNot(HaveOccurred())
}

cfg := dns_server.Config{
Domain: "mesh",
CIDR: "240.0.0.0/24",
ServiceVipEnabled: !tc.whenSkipServiceVips,
}
// When
serviceSet, err := dns.BuildVirtualOutboundMeshView(ctx, rm, !tc.whenSkipServiceVips, "mesh", tc.whenMesh)
allocator, err := dns.NewVIPsAllocator(rm, nil, cfg, tc.whenZone)
Expect(err).NotTo(HaveOccurred())
serviceSet, err := allocator.BuildVirtualOutboundMeshView(ctx, tc.whenMesh)

// Then
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -376,6 +384,7 @@ var _ = DescribeTable("outboundView",
givenResources: map[model.ResourceKey]model.Resource{
model.WithMesh("default", "ingress-1"): &mesh.ZoneIngressResource{
Spec: &mesh_proto.ZoneIngress{
Zone: "zone2",
Networking: &mesh_proto.ZoneIngress_Networking{Port: 1000, AdvertisedPort: 1000, AdvertisedAddress: "127.0.0.1", Address: "127.0.0.1"},
AvailableServices: []*mesh_proto.ZoneIngress_AvailableService{
{
Expand Down Expand Up @@ -404,13 +413,56 @@ var _ = DescribeTable("outboundView",
},
},
whenMesh: "mesh",
whenZone: "zone1",
thenHostnameEntries: []vips.HostnameEntry{vips.NewServiceEntry("srv1"), vips.NewServiceEntry("srv2")},
thenOutbounds: map[vips.HostnameEntry][]vips.OutboundEntry{
vips.NewServiceEntry("srv1"): {
{TagSet: map[string]string{mesh_proto.ServiceTag: "srv1"}, Origin: "service"},
},
},
}),
Entry("zone ingress from own zone is ignored", outboundViewTestCase{
givenResources: map[model.ResourceKey]model.Resource{
model.WithMesh("mesh", "dp1"): &mesh.DataplaneResource{Spec: dp("service1", "service2")},
model.WithMesh("default", "ingress-1"): &mesh.ZoneIngressResource{
Spec: &mesh_proto.ZoneIngress{
Zone: "zone1",
Networking: &mesh_proto.ZoneIngress_Networking{Port: 1000, AdvertisedPort: 1000, AdvertisedAddress: "127.0.0.1", Address: "127.0.0.1"},
AvailableServices: []*mesh_proto.ZoneIngress_AvailableService{
{
Mesh: "other-mesh",
Tags: map[string]string{
mesh_proto.ServiceTag: "srv1",
},
Instances: 2,
},
{
Mesh: "mesh",
Tags: map[string]string{
mesh_proto.ServiceTag: "srv1",
},
Instances: 2,
},
{
Mesh: "mesh",
Tags: map[string]string{
mesh_proto.ServiceTag: "srv2",
},
Instances: 2,
},
},
},
},
},
whenMesh: "mesh",
whenZone: "zone1",
thenHostnameEntries: []vips.HostnameEntry{vips.NewServiceEntry("service1"), vips.NewServiceEntry("service2")},
thenOutbounds: map[vips.HostnameEntry][]vips.OutboundEntry{
vips.NewServiceEntry("service1"): {
{TagSet: map[string]string{mesh_proto.ServiceTag: "service1"}, Origin: "service"},
},
},
}),
Entry("virtual outbound simple", outboundViewTestCase{
givenResources: map[model.ResourceKey]model.Resource{
model.WithMesh("mesh", "dp1-a"): &mesh.DataplaneResource{Spec: dpWithTags(map[string]string{mesh_proto.ServiceTag: "service1", "instance": "a", "port": "9000"})},
Expand Down
5 changes: 5 additions & 0 deletions pkg/plugins/runtime/k8s/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,15 @@ func addDNS(mgr kube_ctrl.Manager, rt core_runtime.Runtime, converter k8s_common
if rt.Config().Mode == config_core.Global {
return nil
}
zone := ""
if rt.Config().Multizone != nil && rt.Config().Multizone.Zone != nil {
zone = rt.Config().Multizone.Zone.Name
}
vipsAllocator, err := dns.NewVIPsAllocator(
rt.ResourceManager(),
rt.ConfigManager(),
*rt.Config().DNSServer,
zone,
)
if err != nil {
return err
Expand Down
5 changes: 5 additions & 0 deletions pkg/plugins/runtime/universal/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ func (p *plugin) Customize(rt core_runtime.Runtime) error {
}

func addDNS(rt core_runtime.Runtime) error {
zone := ""
if rt.Config().Multizone != nil && rt.Config().Multizone.Zone != nil {
zone = rt.Config().Multizone.Zone.Name
}
vipsAllocator, err := dns.NewVIPsAllocator(
rt.ReadOnlyResourceManager(),
rt.ConfigManager(),
*rt.Config().DNSServer,
zone,
)
if err != nil {
return err
Expand Down

0 comments on commit a066dca

Please sign in to comment.