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

chore(kuma-cp) use resource types for DataplaneInsight tracking #2324

Merged
merged 4 commits into from
Jul 12, 2021
Merged

chore(kuma-cp) use resource types for DataplaneInsight tracking #2324

merged 4 commits into from
Jul 12, 2021

Conversation

jpeach
Copy link
Contributor

@jpeach jpeach commented Jul 7, 2021

Summary

When we track the Dataplane status, we could be using that to generate
a DataplaneInsight or a ZoneIngressInsight. Previously, we used the
presence of a mesh field to distinguish these cases, but we can take
advantage of the fact that the xDS metadata round-trips the resource
protobuf through the node metadata to determine the exact type when
the we start tracking the xDS client stream. This has the additional
property of not trying to create mis-matched insight resources for any
other types of resources, which ultimately results in errors from the
DataplaneInsightManager.

Full changelog

N/A

Issues resolved

N/A

Documentation

N/A

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.

This ought to be backwards compatible (thanks integration tests!). We can handle node metadata from old bootstrap metadata as well as from the newly consistent metadata.

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2021

Codecov Report

Merging #2324 (d29cf3a) into master (9ab0bf4) will increase coverage by 0.37%.
The diff coverage is 45.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2324      +/-   ##
==========================================
+ Coverage   50.50%   50.88%   +0.37%     
==========================================
  Files         875      879       +4     
  Lines       40794    40844      +50     
==========================================
+ Hits        20603    20782     +179     
+ Misses      18201    18041     -160     
- Partials     1990     2021      +31     
Impacted Files Coverage Δ
pkg/test/matchers/proto.go 15.62% <0.00%> (ø)
pkg/xds/server/v3/components.go 82.00% <16.66%> (+0.36%) ⬆️
pkg/xds/server/callbacks/dataplane_status_sink.go 58.92% <38.70%> (-10.12%) ⬇️
...g/xds/server/callbacks/dataplane_status_tracker.go 97.43% <83.33%> (+5.33%) ⬆️
api/internal/util/proto/proto.go 62.50% <0.00%> (ø)
...kg/plugins/resources/k8s/native/pkg/test/within.go 100.00% <0.00%> (ø)
...k8s/native/controllers/proxytemplate_controller.go 0.00% <0.00%> (ø)
api/internal/util/proto/types.go 100.00% <0.00%> (ø)
api/observability/v1/mads.pb.go 35.94% <0.00%> (+1.30%) ⬆️
pkg/core/xds/metadata.go 63.76% <0.00%> (+1.44%) ⬆️
... and 13 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 9ab0bf4...d29cf3a. Read the comment docs.

When we track the Dataplane status, we could be using that to generate
a DataplaneInsight or a ZoneIngressInsight. Previously, we used the
presence of a mesh field to distinguish these cases, but we can take
advantage of the fact that the xDS metadata round-trips the resource
protobuf through the node metadata to determine the exact type when
the we start tracking the xDS client stream. This has the additional
property of not trying to create mis-matched insight resources for any
other types of resources, which ultimately results in errors from the
DataplaneInsightManager.

Signed-off-by: James Peach <james.peach@konghq.com>
Signed-off-by: James Peach <james.peach@konghq.com>
@jpeach jpeach marked this pull request as ready for review July 7, 2021 23:04
@jpeach jpeach requested a review from a team as a code owner July 7, 2021 23:04
@jpeach jpeach merged commit 82c6818 into kumahq:master Jul 12, 2021
@jpeach jpeach deleted the chore/dataplane-insight-types branch July 12, 2021 01:27
mergify bot pushed a commit that referenced this pull request Jul 12, 2021
When we track the Dataplane status, we could be using that to generate
a DataplaneInsight or a ZoneIngressInsight. Previously, we used the
presence of a mesh field to distinguish these cases, but we can take
advantage of the fact that the xDS metadata round-trips the proxy type
through the node metadata to determine the right resource type when the
we start tracking the xDS client stream

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit 82c6818)
jpeach added a commit that referenced this pull request Jul 12, 2021
… (#2349)

When we track the Dataplane status, we could be using that to generate
a DataplaneInsight or a ZoneIngressInsight. Previously, we used the
presence of a mesh field to distinguish these cases, but we can take
advantage of the fact that the xDS metadata round-trips the proxy type
through the node metadata to determine the right resource type when the
we start tracking the xDS client stream

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit 82c6818)

Co-authored-by: James Peach <james.peach@konghq.com>
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