-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Scaffold code in cmd/manager/main.go
for Go operators and add logic to Ansible/Helm operators to handle [multinamespace caching]
#2522
Scaffold code in cmd/manager/main.go
for Go operators and add logic to Ansible/Helm operators to handle [multinamespace caching]
#2522
Conversation
8553e66
to
4bb3aa1
Compare
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.
Overall a nice feature for users, needs work though.
@joelanford @fabianvf anything else we need to do for Helm/Ansible multinamespacing?
@@ -6,6 +6,7 @@ | |||
- Added `pkg/status` with several new types and interfaces that can be used in `Status` structs to simplify handling of [status conditions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties). ([#1143](https://github.com/operator-framework/operator-sdk/pull/1143)) | |||
- Added support for relative Ansible roles and playbooks paths in the Ansible operator's file. ([#2273](https://github.com/operator-framework/operator-sdk/pull/2273)) | |||
- Add Prometheus metrics support to Ansible-based operators. ([#2179](https://github.com/operator-framework/operator-sdk/pull/2179)) | |||
- Scaffold code in `cmd/manager/main.go` for Go operators and add logic to Ansible/Helm operators to handle [multinamespace caching](https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/cache#MultiNamespacedCacheBuilder) if `WATCH_NAMESPACE` contains multiple namespaces. ([#2522](https://github.com/operator-framework/operator-sdk/pull/2522)) | |||
|
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.
Hi @dmesser have you any objection to solve the scenario with?
It is also related to the option targetNamespaces that allows MultiNamespace via the OperatorGroup.
Example:
targetNamespaces:
- ns1
- ns2
The scenario is described in #2361
Also, please see that it address common scenarios such as: #2494
c/c @joelanford
cmd/manager/main.go
for Go operators and add logic to Ansible/Helm operators to handle [multinamespace caching]
b24aa8f
to
39ce485
Compare
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.
/lgtm
Would like one more set of eyes on this first @hasbro17 @jmccormick2001.
internal/scaffold/cmd.go
Outdated
options.NewCache = cache.MultiNamespacedCacheBuilder(strings.Split(namespace, ",")) | ||
} | ||
|
||
// Create a new Cmd to provide shared dependencies and start components |
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.
// Create a new Cmd to provide shared dependencies and start components | |
// Create a new manager to provide shared dependencies and start components |
Whats Cmd
?
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.
LGTM with a minor typo in the scaffold comments.
internal/scaffold/cmd_test.go
Outdated
options.NewCache = cache.MultiNamespacedCacheBuilder(strings.Split(namespace, ",")) | ||
} | ||
|
||
// Create a new Cmd to provide shared dependencies and start components |
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.
// Create a new Cmd to provide shared dependencies and start components | |
// Create a new manager to provide shared dependencies and start components |
pkg/ansible/run.go
Outdated
options.Namespace = metav1.NamespaceAll | ||
} | ||
|
||
// Create a new Cmd to provide shared dependencies and start components |
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.
// Create a new Cmd to provide shared dependencies and start components | |
// Create a new manager to provide shared dependencies and start components |
New changes are detected. LGTM label has been removed. |
Description of the change:
Motivation for the change:
Integration with OLM: See that OLM allows and config the MultiNamespace via the option
targetNamespaces
via the OperatorGroup. We are also in the olm commands setting these values in the WATCH-NAMESPACE EnvVar.Address the requirements requested in tasks such as Bug creating metric service monitors when watching a different namespace #2494, Set namespsace for metrics to WATCH_NAMESPACE #2078, (which are a very common scenario in the channels: I as an operator dev, would like to deploy the operator in the nsA and WATCH the resources in the nsB and do not all cluster ),
Closes #2361