-
Notifications
You must be signed in to change notification settings - Fork 1.8k
update go tutorial docs to reflect call to NewManager func #3651
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
Conversation
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
good catcher 👍
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
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.
There is no controller-runtime.pkg/manager.NewManager() constructor, perhaps you're thinking of controller-runtime.NewManager()? If so, we need to also update the import alias from manager to ctrl, which is what main.go uses.
Canceling my review as @estroz pointed out, there's a problem here.
|
Hi @estroz, The first comment shows a little confuse. But the change here is just to fix the constructor in the examples to be consistent with the new code that is scaffolded. Replace |
|
I'm seeing the following import in a main.go from operator-sdk 0.18 (i.e. "old state") import (
// ... other imports here
"sigs.k8s.io/controller-runtime/pkg/manager"
)I see it used to create a manager like so: // Create a new manager to provide shared dependencies and start components
mgr, err := manager.New(cfg, options)
if err != nil {
log.Error(err, "")
os.Exit(1)
}New state does refer to the import of controller-runtime as Can you confirm? If so, I'll push straight away. |
| The Manager can restrict the namespace that all controllers will watch for resources: | ||
| ```Go | ||
| mgr, err := manager.New(cfg, manager.Options{Namespace: namespace}) | ||
| mgr, err := manager.NewManager(cfg, manager.Options{Namespace: namespace}) |
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.
| mgr, err := manager.NewManager(cfg, manager.Options{Namespace: namespace}) | |
| mgr, err := ctrl.NewManager(cfg, manager.Options{Namespace: namespace}) |
| By default this will be the namespace that the operator is running in. To watch all namespaces leave the namespace option empty: | ||
| ```Go | ||
| mgr, err := manager.New(cfg, manager.Options{Namespace: ""}) | ||
| mgr, err := manager.NewManager(cfg, manager.Options{Namespace: ""}) |
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.
| mgr, err := manager.NewManager(cfg, manager.Options{Namespace: ""}) | |
| mgr, err := ctrl.NewManager(cfg, manager.Options{Namespace: ""}) |
| var namespaces []string // List of Namespaces | ||
| // Create a new Cmd to provide shared dependencies and start components | ||
| mgr, err := manager.New(cfg, manager.Options{ | ||
| mgr, err := manager.NewManager(cfg, manager.Options{ |
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.
| mgr, err := manager.NewManager(cfg, manager.Options{ | |
| mgr, err := ctrl.NewManager(cfg, manager.Options{ |
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.
Got it. Updating and pushing momentarily.
Signed-off-by: Jose R. Gonzalez <[email protected]>
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.
Thanks!
/lgtm
|
Should be good to go! |
Signed-off-by: Jose R. Gonzalez [email protected]
Description of the change:
With v1.0, the scaffolding creates a manager with some default options using
"sigs.k8s.io/controller-runtime".NewManager()(changed from"sigs.k8s.io/controller-runtime/pkg/manager".New()) This seems to be accounted for in other places in the docs.Motivation for the change:
Thought it might help with consistency across the documentation. Not sure if the Tutorial in its current form was going to stick around but figured I'd send this PR either way and we just close if it not.
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs