feat(*) add Zone resource to register Remotes to Global #895

nickolaev · 2020-07-10T11:52:29Z

Summary

Adding a zone resource.

Full changelog

[Implement ...]
[Fix ...]

Issues resolved

Fix #XXX

Documentation

Link to the website documentation PR

Signed-off-by: Nikolay Nikolaev <[email protected]>

… with delay Signed-off-by: Nikolay Nikolaev <[email protected]>

lobkovilya · 2020-07-14T11:41:54Z

app/kumactl/cmd/get/get_zone.go

+				if store.IsResourceNotFound(err) {
+					return errors.Errorf("No resources found in %s mesh", currentMesh)
+				}
+				return errors.Wrapf(err, "failed to get mesh %s", currentMesh)


Probably meant failed to get zone?

lobkovilya · 2020-07-14T11:46:45Z

api/system/v1alpha1/zone.proto

+  // Configure the Remote Control Plane
+  message RemoteControlPlane {
+    // The public load balancer address of the Remote Control Plane KDS
+    string publicAddress = 1;


Isn't it clear that the address should be "public"? Maybe just "address" is enough?

Well, that was the name we agreed in the spec. I don't have any particular attachment to publicAddress, so we can go either way. @jakubdyszkiewicz what do you think?

I like just address

lobkovilya · 2020-07-14T12:39:50Z

pkg/core/resources/apis/system/zone.go

@@ -0,0 +1,82 @@
+package system


I know that we don't have validators for system API, but every entity in mesh API has it. I think it is useful to have zone_validator.go and check that all IP addresses have correct format

lobkovilya · 2020-07-14T12:44:17Z

pkg/kds/global/components_test.go

+					PublicAddress: "grpc://192.168.0.1",
+				},
+				Ingress: &system_proto.Zone_Ingress{
+					PublicAddress: "192.168.0.2",


This is the test, so not really important, but this is an argument for validation. Ingress address should look like host:port

jakubdyszkiewicz · 2020-07-14T14:17:15Z

app/kumactl/cmd/get/get_zones.go

+
+				return []string{
+					zone.GetMeta().GetName(), // NAME
+					table.TimeSince(zone.GetMeta().GetModificationTime(), rootTime), // AGE


can we print remoteCP and ingress address?

Remote CP will be removed in the next PR. Adding the Ingress address only.

jakubdyszkiewicz · 2020-07-14T14:17:49Z

api/system/v1alpha1/zone.proto

+  // Configure the Remote Control Plane
+  message RemoteControlPlane {
+    // The public load balancer address of the Remote Control Plane KDS
+    string publicAddress = 1;


I like just address

pkg/config/app/kuma-cp/kuma-cp.defaults.yaml

pkg/kds/global/components.go

jakubdyszkiewicz · 2020-07-14T14:30:21Z

test/e2e/kuma_deploy_hybrid_test.go

@@ -13,7 +13,7 @@ import (
 	. "github.com/kumahq/kuma/test/framework"
 )

-var _ = Describe("Test Kubernetes/Universal deployment", func() {
+var _ = XDescribe("Test Kubernetes/Universal deployment", func() {


This one will break, we need the bi-directional KDS to make this work.
Simply - put, Universal is run from memory so resources do not persist during restarts, and we have no way to use the Zone resources unless we restart (which will erase the resource from memory)

jakubdyszkiewicz · 2020-07-14T14:31:17Z

test/e2e/kuma_deploy_universal_test.go

@@ -13,7 +14,7 @@ import (
 	. "github.com/kumahq/kuma/test/framework"
 )

-var _ = Describe("Test Universal deployment", func() {
+var _ = XDescribe("Test Universal deployment", func() {


See my response for the other one.

Signed-off-by: Nikolay Nikolaev <[email protected]>

jakubdyszkiewicz · 2020-07-15T11:25:34Z

One more thing to be aware of. This resource seems to be bound to default mesh. When default mesh is deleted it will be deleted. We cannot create it without default Mesh.
I think it's more of a Admin's resource (like Secret) that belongs to Admin Server. IMHO we need a concept of Resource which is not Mesh bound (recently introduced Config is a second example).

But let's keep it this way for now. I think we might get a more clear picture when joining the Admin server with the API Server and introducing RBAC.

nickolaev · 2020-07-15T11:56:21Z

@jakubdyszkiewicz completely agree. Having a Zone resource belonging to a Mesh sounds wrong. I have not found a way to remove it. It is kind of hidden, because kumactl apply will omit the check, but still behind the scenes, the mesh is set to "default".

lobkovilya · 2020-07-16T04:18:10Z

pkg/core/resources/apis/system/zone_validator.go

+	if err != nil {
+		verr.AddViolation("address", "invalid address")
+	} else {
+		if host == "" {


Kuma validates resources in a way that it tries to find as many errors as possible. So we don't need else here:

if host == "" { verr.AddViolation("address", "host has to be explicitly specified") } if port == "" { verr.AddViolation("address", "port has to be explicitly specified") }

Agree, but then if there is an error, host and port will be empty for sure.

Yes, but I'm talking about if-branch where err == nil

# Conflicts: # app/kumactl/pkg/install/k8s/control-plane/templates_vfsdata.go

Signed-off-by: Nikolay Nikolaev <[email protected]>

Nikolay Nikolaev added 19 commits July 10, 2020 12:22

feat(kuma-cp) add Zone API

8e890e8

Signed-off-by: Nikolay Nikolaev <[email protected]>

tbd

fd8afa4

Signed-off-by: Nikolay Nikolaev <[email protected]>

feat(*) support zones API

3e3b0c3

Signed-off-by: Nikolay Nikolaev <[email protected]>

feat(kumact) add get zone and get zones

fe44366

Signed-off-by: Nikolay Nikolaev <[email protected]>

test(kumactl) get zone/zones

1154a94

Signed-off-by: Nikolay Nikolaev <[email protected]>

fix(*) rename clusters to zones

8bdd1c9

Signed-off-by: Nikolay Nikolaev <[email protected]>

test(e2e) adjust for zones

f864cb1

Signed-off-by: Nikolay Nikolaev <[email protected]>

fix(*) more fixes

b6d63a9

Signed-off-by: Nikolay Nikolaev <[email protected]>

fix(kuma-cp) migrate KDS

e5da951

Signed-off-by: Nikolay Nikolaev <[email protected]>

Merge remote-tracking branch 'origin/master' into feat/zone_resource

a97ac62

fix(*) renames after migrate to kumahq

f442467

Signed-off-by: Nikolay Nikolaev <[email protected]>

fix(*) e2e tests

5fac375

Signed-off-by: Nikolay Nikolaev <[email protected]>

Merge remote-tracking branch 'origin/master' into feat/zone_resource

c6d05fd

fix(*) rename crds

2015fc6

Signed-off-by: Nikolay Nikolaev <[email protected]>

fix(*) update CRDs for zones generation

bb6b4a0

Signed-off-by: Nikolay Nikolaev <[email protected]>

fix(kuma-cp) fix zones startup

fa470ea

Signed-off-by: Nikolay Nikolaev <[email protected]>

test(e2e) SetLbAddress

aff19b4

Signed-off-by: Nikolay Nikolaev <[email protected]>

test(e2e) poller cleanups

bc978d0

Signed-off-by: Nikolay Nikolaev <[email protected]>

test(e2e) deprecate Hybrid and Universal, make K8s Global/Remote work…

6ba3864

… with delay Signed-off-by: Nikolay Nikolaev <[email protected]>

nickolaev marked this pull request as ready for review July 14, 2020 09:25

nickolaev requested a review from a team as a code owner July 14, 2020 09:25

lobkovilya reviewed Jul 14, 2020

View reviewed changes

jakubdyszkiewicz reviewed Jul 14, 2020

View reviewed changes

fix(*) review remarks

50069a2

Signed-off-by: Nikolay Nikolaev <[email protected]>

lobkovilya reviewed Jul 16, 2020

View reviewed changes

Nikolay Nikolaev added 2 commits July 16, 2020 09:33

Merge remote-tracking branch 'origin/master' into feat/zone_resource

ec0e917

# Conflicts: # app/kumactl/pkg/install/k8s/control-plane/templates_vfsdata.go

fix(*) review

7d4e04e

Signed-off-by: Nikolay Nikolaev <[email protected]>

jakubdyszkiewicz approved these changes Jul 16, 2020

View reviewed changes

nickolaev merged commit 3682f7a into master Jul 16, 2020

nickolaev deleted the feat/zone_resource branch July 31, 2020 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(*) add Zone resource to register Remotes to Global #895

feat(*) add Zone resource to register Remotes to Global #895

nickolaev commented Jul 10, 2020

lobkovilya Jul 14, 2020

lobkovilya Jul 14, 2020

nickolaev Jul 14, 2020

jakubdyszkiewicz Jul 14, 2020

lobkovilya Jul 14, 2020

lobkovilya Jul 14, 2020

jakubdyszkiewicz Jul 14, 2020

nickolaev Jul 14, 2020

jakubdyszkiewicz Jul 14, 2020

jakubdyszkiewicz Jul 14, 2020

nickolaev Jul 14, 2020

jakubdyszkiewicz Jul 14, 2020

nickolaev Jul 14, 2020

jakubdyszkiewicz commented Jul 15, 2020

nickolaev commented Jul 15, 2020

lobkovilya Jul 16, 2020

nickolaev Jul 16, 2020

lobkovilya Jul 16, 2020

feat(*) add Zone resource to register Remotes to Global #895

feat(*) add Zone resource to register Remotes to Global #895

Conversation

nickolaev commented Jul 10, 2020

Summary

Full changelog

Issues resolved

Documentation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubdyszkiewicz commented Jul 15, 2020

nickolaev commented Jul 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment