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.

Sign up for GitHub

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

Jump to bottom

pkg/ansible; Add watching of dependent resources #857

Merged
merged 13 commits into from
Jan 2, 2019
Merged

pkg/ansible; Add watching of dependent resources #857

merged 13 commits into from
Jan 2, 2019

Conversation

dymurray
Copy link
Contributor

@dymurray dymurray commented Dec 18, 2018 •
edited
Loading

Description of the change:
Adding support for the Ansible Operator to keep track of resources that are created by adding them for the controller to watch if we have already created a controller object for the specific GVK.

Motivation for the change:
Performance Enhancement.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 18, 2018
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2018
@dymurray dymurray force-pushed the depres branch 2 times, most recently from eff1b0e to 0622481 Compare December 18, 2018 00:43
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 18, 2018
shawn-hurley
shawn-hurley approved these changes Dec 18, 2018
View reviewed changes
Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

Sorry most of this is nits,

pkg/ansible/controller/controller.go
@@ -63,6 +69,13 @@ func Add(mgr manager.Manager, options Options) {
ManageStatus: options.ManageStatus,
}

if mgr.GetScheme().IsVersionRegistered(schema.GroupVersion{
Copy link
Member

Choose a reason for hiding this comment

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

👍

pkg/ansible/controller/controller.go Outdated Show resolved Hide resolved
pkg/ansible/proxy/proxy.go Outdated Show resolved Hide resolved
pkg/ansible/proxy/proxy.go Outdated Show resolved Hide resolved
pkg/ansible/controller/controller.go Outdated Show resolved Hide resolved
pkg/ansible/proxy/proxy.go Outdated Show resolved Hide resolved
pkg/ansible/proxy/proxy.go Outdated Show resolved Hide resolved
shawn-hurley
shawn-hurley requested changes Dec 18, 2018
View reviewed changes
Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

Nits and some functionality stuff

@dymurray
Copy link
Contributor Author

Travis seems to have a transient error unrelated to my PR

@openshift-ci-robot
Copy link

@dymurray: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@shawn-hurley shawn-hurley added the language/ansible Issue is related to an Ansible operator project label Dec 19, 2018
@dymurray dymurray changed the title [WIP] pkg/ansible; Add watching of dependent resources pkg/ansible; Add watching of dependent resources Dec 19, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 19, 2018
shawn-hurley
shawn-hurley reviewed Dec 19, 2018
View reviewed changes
pkg/ansible/controller/controller.go Outdated Show resolved Hide resolved
pkg/ansible/operator/operator.go Outdated Show resolved Hide resolved
pkg/ansible/proxy/proxy.go Outdated Show resolved Hide resolved
shawn-hurley
shawn-hurley requested changes Dec 19, 2018
View reviewed changes
Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

LGTM

/cc @mhrivnak can you take a second look?

mhrivnak
mhrivnak approved these changes Dec 19, 2018
View reviewed changes
commands/operator-sdk/cmd/up/local.go Outdated Show resolved Hide resolved
@dymurray
Copy link
Contributor Author

/retest

@openshift-ci-robot
Copy link

@dymurray: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dymurray
Copy link
Contributor Author

Travis failure seems to be a transient networking problem

@hasbro17
Copy link
Contributor

I've manually reset the travis job.

@shawn-hurley shawn-hurley merged commit fd86f9d into operator-framework:master Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shawn-hurley shawn-hurley shawn-hurley approved these changes

@mhrivnak mhrivnak mhrivnak approved these changes

Assignees
No one assigned
Labels
language/ansible Issue is related to an Ansible operator project size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dymurray @openshift-ci-robot @hasbro17 @mhrivnak @shawn-hurley