-
Notifications
You must be signed in to change notification settings - Fork 327
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
kuma-cp: use Controller to create default Mesh on k8s #298
Conversation
1aaa16d
to
f5fb69f
Compare
pkg/core/bootstrap/bootstrap.go
Outdated
return errors.Errorf("unknown environment type %s", env) | ||
} | ||
// schedule a one-off create default Mesh operation | ||
return runtime.Add(core_runtime.ComponentFunc(func(stop <-chan struct{}) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simplified. We were doing default mesh via Component just because on K8S we had to wait for the cache. Now we can just call CreateDefaultMesh
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
func (d *Defaults) MeshProto() (v1alpha1.Mesh, error) { | ||
func (d *Defaults) MeshProto() v1alpha1.Mesh { | ||
mesh, err := d.parseMesh() | ||
util_error.MustNot(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it is ok to use this? I thought the whole point of golang error handling is to always pass error and log it at app root level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's acceptable when an error indicates a programming mistake.
In this particular case, configuration must be validated once on application start up.
If there will be ever an error at this point, it's definitely a programming mistake.
}), | ||
}). | ||
Complete(r) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to introduce high level integration tests?
Given the things described in Motivation
I feel like it would be beneficial.
Not requiring it right now, but maybe create a github issue for it?
changes:
Mesh
creation to make the logic re-usable from a k8s ControllerMesh
on k8smotivation:
Mesh
resourceMesh
from a reconciliation loop