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

feat(ansible): make ansible verbosity configurable #2087

Merged
merged 3 commits into from
Oct 23, 2019

Conversation

djzager
Copy link
Contributor

@djzager djzager commented Oct 21, 2019

Make it possible to configure the verbosity of ansible-runner command
invocations via command line flags. The default value should be 2 to
avoid unexpected changes in behavior. Also, like maxWorkers the
ansibleVerbosity can be overridden per GVK with an environment
variable of the form ANSIBLE_VERBOSITY_$KIND_$GROUP.

This PR:

  • Adds new command line flag ansible-verbosity for Ansible based
    operators
  • Add internal cmdFuncType to runner pkg for use when constructing
    command functions

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 21, 2019
@djzager
Copy link
Contributor Author

djzager commented Oct 21, 2019

This is the last part of the work broken up from #1852

type cmdFuncType func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd

func playbookCmdFunc(verbosity, path string) cmdFuncType {
return func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may be worth creating an issue to make it possible to configure the ansibleVerbosity via an annotation on the Custom Resource. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I like that idea a lot actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asmacdo asmacdo added the language/ansible Issue is related to an Ansible operator project label Oct 21, 2019
Copy link
Member

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 22, 2019
@djzager
Copy link
Contributor Author

djzager commented Oct 22, 2019

@camilamacedo86 @jmrodri You both had a look at #1852 . This is the last PR broken up from that.

Copy link
Member

@asmacdo asmacdo left a comment

Choose a reason for hiding this comment

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

Request 1 minor change.

pkg/ansible/watches/watches.go Outdated Show resolved Hide resolved
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

minor comment changes, otherwise looks good.

doc/ansible/dev/advanced_options.md Show resolved Hide resolved

Setting the verbosity at which `ansible-runner` is run controls how verbose the
output of `ansible-playbook` will be. The normal rules for verbosity apply here
where higher values means more verbose output. Acceptable values range from 0
Copy link
Member

Choose a reason for hiding this comment

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

tense: I think it should read "higher values mean more"

doc/ansible/dev/advanced_options.md Show resolved Hide resolved
pkg/ansible/run.go Show resolved Hide resolved
pkg/ansible/watches/watches.go Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 22, 2019
@djzager
Copy link
Contributor Author

djzager commented Oct 22, 2019

Updates made. Let me know if I missed anything.

Make it possible to configure the verbosity of `ansible-runner` command
invocations via command line flags. The default value should be 2 to
avoid unexpected changes in behavior. Also, like `maxWorkers` the
`ansibleVerbosity` can be overridden per GVK with an environment
variable of the form `ANSIBLE_VERBOSITY_$KIND_$GROUP`.

This PR:

- Adds new command line flag `ansible-verbosity` for Ansible based
  operators
- Add internal `cmdFuncType` to runner pkg for use when constructing
  command functions
- word choice in advanced_options
- refer to correct environment key in getAnsibleVerbosity()
@djzager
Copy link
Contributor Author

djzager commented Oct 22, 2019

Rebased and updated the changelog entry to put this with the unreleased.

@jmrodri jmrodri requested a review from asmacdo October 23, 2019 00:07
@jmrodri
Copy link
Member

jmrodri commented Oct 23, 2019

/retest

@asmacdo
Copy link
Member

asmacdo commented Oct 23, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2019
@djzager
Copy link
Contributor Author

djzager commented Oct 23, 2019

/retest

@asmacdo
Copy link
Member

asmacdo commented Oct 23, 2019

/retest

@asmacdo
Copy link
Member

asmacdo commented Oct 23, 2019

/retest

@asmacdo
Copy link
Member

asmacdo commented Oct 23, 2019

/test e2e-aws-go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/ansible Issue is related to an Ansible operator project lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants