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.

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

fix(dns): Ensure outbounds are set when migrating from old to new #2698

Merged
merged 2 commits into from
Aug 27, 2021

Conversation

lahabana
Copy link
Contributor

Summary

There's a bug when migrating from one version to the other.
This is caused by the fact that we don't set OutboundEntry when
migrating from old to new format.

Full changelog

  • Adds validation on VirtualOutboundView to avoid this in the future
  • Adds an OutboundEntry for Services with a single tag
  • Adds an OutboundEntry for ExternalServices by fetching back the original
    virtual service to retrieve its serviceName

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

There's a bug when migrating from one version to the other.
This is caused by the fact that we don't set OutboundEntry when
migrating from old to new format.

This patch:

- Adds validation on VirtualOutboundView to avoid this in the future
- Adds an OutboundEntry for Services with a single tag
- Adds an OutboundEntry for ExternalServices by fetching back the original
virtual service to retrieve its serviceName

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2021

Codecov Report

Merging #2698 (198e15a) into master (d1301a2) will increase coverage by 0.10%.
The diff coverage is 85.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2698      +/-   ##
==========================================
+ Coverage   51.94%   52.04%   +0.10%     
==========================================
  Files         871      871              
  Lines       49518    49543      +25     
==========================================
+ Hits        25721    25784      +63     
+ Misses      21716    21671      -45     
- Partials     2081     2088       +7     
Impacted Files Coverage Δ
pkg/dns/vips/virtual_outbound_view.go 90.47% <75.00%> (-2.75%) ⬇️
pkg/dns/vips/persistence.go 70.70% <86.66%> (+5.32%) ⬆️
pkg/dns/vips_allocator.go 74.00% <100.00%> (ø)
pkg/xds/generator/direct_access_proxy_generator.go 83.90% <0.00%> (+1.14%) ⬆️
pkg/insights/resyncer.go 67.66% <0.00%> (+1.50%) ⬆️
pkg/mads/server/server.go 85.18% <0.00%> (+2.77%) ⬆️
pkg/plugins/resources/memory/store.go 82.06% <0.00%> (+4.34%) ⬆️
pkg/core/resources/manager/cache.go 84.41% <0.00%> (+5.19%) ⬆️
pkg/core/bootstrap/autoconfig.go 54.46% <0.00%> (+8.03%) ⬆️
.../core/managers/apis/ratelimit/ratelimit_manager.go 45.16% <0.00%> (+9.67%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1301a2...198e15a. Read the comment docs.

}
srvByHost := map[string]*core_mesh.ExternalServiceResource{}
for _, s := range externalServices.Items {
srvByHost[s.Spec.GetHost()] = s
Copy link
Contributor

Choose a reason for hiding this comment

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

The host is not unique, I think you have to use networking.address here

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana merged commit 203c072 into kumahq:master Aug 27, 2021
mergify bot pushed a commit that referenced this pull request Aug 27, 2021
)

* fix(dns): Ensure outbounds are set when migrating from old to new

There's a bug when migrating from one version to the other.
This is caused by the fact that we don't set OutboundEntry when
migrating from old to new format.

This patch:

- Adds validation on VirtualOutboundView to avoid this in the future
- Adds an OutboundEntry for Services with a single tag
- Adds an OutboundEntry for ExternalServices by fetching back the original
virtual service to retrieve its serviceName

Signed-off-by: Charly Molter <charly.molter@konghq.com>
(cherry picked from commit 203c072)
lahabana added a commit that referenced this pull request Aug 27, 2021
) (#2699)

* fix(dns): Ensure outbounds are set when migrating from old to new

There's a bug when migrating from one version to the other.
This is caused by the fact that we don't set OutboundEntry when
migrating from old to new format.

This patch:

- Adds validation on VirtualOutboundView to avoid this in the future
- Adds an OutboundEntry for Services with a single tag
- Adds an OutboundEntry for ExternalServices by fetching back the original
virtual service to retrieve its serviceName

Signed-off-by: Charly Molter <charly.molter@konghq.com>
(cherry picked from commit 203c072)

Co-authored-by: Charly Molter <charly.molter@konghq.com>
nikita15p pushed a commit to nikita15p/kuma that referenced this pull request Sep 28, 2021
…mahq#2698)

* fix(dns): Ensure outbounds are set when migrating from old to new

There's a bug when migrating from one version to the other.
This is caused by the fact that we don't set OutboundEntry when
migrating from old to new format.

This patch:

- Adds validation on VirtualOutboundView to avoid this in the future
- Adds an OutboundEntry for Services with a single tag
- Adds an OutboundEntry for ExternalServices by fetching back the original
virtual service to retrieve its serviceName

Signed-off-by: Charly Molter <charly.molter@konghq.com>
nikita15p pushed a commit to nikita15p/kuma that referenced this pull request Sep 28, 2021
…mahq#2698)

* fix(dns): Ensure outbounds are set when migrating from old to new

There's a bug when migrating from one version to the other.
This is caused by the fact that we don't set OutboundEntry when
migrating from old to new format.

This patch:

- Adds validation on VirtualOutboundView to avoid this in the future
- Adds an OutboundEntry for Services with a single tag
- Adds an OutboundEntry for ExternalServices by fetching back the original
virtual service to retrieve its serviceName

Signed-off-by: Charly Molter <charly.molter@konghq.com>
@lahabana lahabana deleted the fixUpgradePersistence branch March 29, 2024 12:45
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.

None yet

3 participants