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

remote-cp version in zoneInsight #1380

Merged

Conversation

tharun208
Copy link
Contributor

Make remote kds to send cp version in discovery request metadata

Summary

This PR resolves #1364

Full changelog

  • Made remote cp kds to send kuma-cp version in DiscoveryRequest.Node.Metadata
  • Updated zoneInsight proto with kuma-cp` version field.
  • Updated inspect zones cli to return kuma-cp version.

Issues resolved

Fix #1364

Pending: Update kuma-gui zones view with kuma-cp versions

@tharun208 tharun208 requested a review from a team as a code owner January 5, 2021 17:25
@tharun208 tharun208 force-pushed the feat/remote_cp_version_zone_insight branch 4 times, most recently from 3f96cad to d03ed06 Compare January 5, 2021 22:04
@tharun208 tharun208 marked this pull request as draft January 6, 2021 05:43
@tharun208 tharun208 marked this pull request as ready for review January 6, 2021 05:48
Make remote kds to send cp version in discoveryRequest metadata

Fixes kumahq#1364

Signed-off-by: Tharun <rajendrantharun@live.com>
@tharun208 tharun208 force-pushed the feat/remote_cp_version_zone_insight branch from d03ed06 to 299cd75 Compare January 6, 2021 20:03
Signed-off-by: Tharun <rajendrantharun@live.com>
Signed-off-by: Tharun <rajendrantharun@live.com>
@jakubdyszkiewicz
Copy link
Contributor

jakubdyszkiewicz commented Jan 8, 2021

Ok, now I see the value is properly passed and parsed.

But, there is a problem with backward compatibility. I run remote 1.0.3 and see this

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x2aad126]

goroutine 236 [running]:
github.com/kumahq/kuma/pkg/kds/server.readVersion(0x0, 0xc001f15500, 0x3, 0xc0001fc830)
	/Users/jakob/kong/kuma/pkg/kds/server/status_tracker.go:186 +0x26
github.com/kumahq/kuma/pkg/kds/server.(*statusTracker).OnStreamRequest(0xc000879e00, 0x3, 0xc0007f9800, 0x0, 0x0)
	/Users/jakob/kong/kuma/pkg/kds/server/status_tracker.go:126 +0x165
github.com/kumahq/kuma/pkg/util/xds/v2.CallbacksChain.OnStreamRequest(0xc000dfbae0, 0x6, 0xa, 0x3, 0xc0007f9800, 0x1, 0xc00155b790)
	/Users/jakob/kong/kuma/pkg/util/xds/v2/callbacks_chain.go:37 +0x72
github.com/envoyproxy/go-control-plane/pkg/server/sotw/v2.(*server).process(0xc000a0cfc0, 0x2e1f7d58, 0xc001fc0c20, 0xc0006ea9c0, 0x0, 0x0, 0x0, 0x0)
	/Users/jakob/go/pkg/mod/github.com/envoyproxy/go-control-plane@v0.9.8/pkg/server/sotw/v2/server.go:300 +0x1202
github.com/envoyproxy/go-control-plane/pkg/server/sotw/v2.(*server).StreamHandler(0xc000a0cfc0, 0x2e1f7d58, 0xc001fc0c20, 0x0, 0x0, 0x0, 0xc001e29e58)
	/Users/jakob/go/pkg/mod/github.com/envoyproxy/go-control-plane@v0.9.8/pkg/server/sotw/v2/server.go:416 +0xe8
github.com/kumahq/kuma/pkg/kds/server.(*server).StreamKumaResources(0xc0017acd80, 0x39a6300, 0xc001fc0c20, 0xc001f91080, 0xc001623600)
	/Users/jakob/kong/kuma/pkg/kds/server/kds.go:36 +0x8b
github.com/kumahq/kuma/pkg/kds/global.Setup.func1.1(0x397d860, 0xc0017acd80, 0x3998920, 0xc001d3d740, 0x399c640, 0xc001fc0ca0)
	/Users/jakob/kong/kuma/pkg/kds/global/components.go:71 +0x49
created by github.com/kumahq/kuma/pkg/kds/global.Setup.func1
	/Users/jakob/kong/kuma/pkg/kds/global/components.go:70 +0x21f

please add a nil check on metadata itself

func readVersion(metadata *pstruct.Struct, version *system_proto.Version) error {
	if metadata == nil {
		return nil
	}

@tharun208
Copy link
Contributor Author

if metadata == nil {
return nil
}

@jakubdyszkiewicz, we need to add the nil check to the dataplane_status_tracker.go right ?

@jakubdyszkiewicz
Copy link
Contributor

In Envoy case technically we always had some metadata, but ok - let's maybe add it

Signed-off-by: Tharun <rajendrantharun@live.com>
@jakubdyszkiewicz jakubdyszkiewicz merged commit fe761f2 into kumahq:master Jan 13, 2021
mergify bot pushed a commit that referenced this pull request Jan 13, 2021
Signed-off-by: Tharun <rajendrantharun@live.com>
(cherry picked from commit fe761f2)
@tharun208 tharun208 deleted the feat/remote_cp_version_zone_insight branch January 13, 2021 08:58
nickolaev pushed a commit that referenced this pull request Jan 13, 2021
Signed-off-by: Tharun <rajendrantharun@live.com>
(cherry picked from commit fe761f2)

Co-authored-by: Tharun Rajendran <rajendrantharun@live.com>
austince pushed a commit to hvydya/kuma that referenced this pull request Jan 17, 2021
Signed-off-by: Tharun <rajendrantharun@live.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.

Versions of the Remote CP in Global CP
3 participants