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(*) handle resources that are global scoped #1127

Merged
merged 8 commits into from
Nov 10, 2020

Conversation

jakubdyszkiewicz
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz commented Nov 4, 2020

Summary

This PR fixes the fact that Zone is bound to a Mesh.
We have two resources that are not abound to a mesh - Zone and Mesh.
Our core resources has mesh field in all resources.

Before this PR:

  • Mesh - we filled mesh field with name field
  • Zone/ZoneInsights - we filled default in a mesh. We cannot do this, because Zone is not really bound to a Mesh and default Mesh may not be even in the system.

With this PR:

  • I introduced a general assumption in the code that resource can have an empty mesh field if resource is of ScopeGlobal

Full changelog

  • Changed all the code handling that CRUDs Mesh and Zone that the mesh is empty
  • Added migration for postgres that adjusts resources
  • As a result - simplified API Server code a bit.

Issues resolved

Fix #1023

Documentation

  • No docs, internal mechanism only.

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team as a code owner November 4, 2020 15:45
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
mk/run.mk Show resolved Hide resolved
@@ -10,6 +10,9 @@ import (

const (
DefaultMesh = "default"
// NoMesh defines a marker that resource is not bound to a Mesh.
// Resources not bound to a mesh (ScopeGlobal) should have an empty string in Mesh field.
NoMesh = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, can we use some special character/string instead? Not sure if "" is the universal way to designate NoMesh. How is this encoded in YAML/JSON?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Not sure if "" is the universal way to designate NoMesh " Hm, why not?

When string is empty, mesh field is excluded from the JSON/YAML.

Copy link
Contributor

@nickolaev nickolaev left a comment

Choose a reason for hiding this comment

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

I like it. Have a couple of small remarks only.

Copy link
Contributor

@lobkovilya lobkovilya left a comment

Choose a reason for hiding this comment

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

Overall looks great! Just a couple of comments that might improve the store's user experience

app/kumactl/cmd/get/get_mesh.go Show resolved Hide resolved
pkg/core/resources/store/store.go Show resolved Hide resolved
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz merged commit 586e8fe into master Nov 10, 2020
@jakubdyszkiewicz jakubdyszkiewicz deleted the chore/resources-scope-global branch November 10, 2020 09:19
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.

Zone is mesh-scoped
3 participants