Skip to content

Disable fact gathering by default#3933

Merged
fabianvf merged 4 commits into
operator-framework:masterfrom
fabianvf:ansible-disable-default-gathering
Sep 28, 2020
Merged

Disable fact gathering by default#3933
fabianvf merged 4 commits into
operator-framework:masterfrom
fabianvf:ansible-disable-default-gathering

Conversation

@fabianvf
Copy link
Copy Markdown
Member

@fabianvf fabianvf commented Sep 22, 2020

Description of the change:
With the migration to kubebuilder style scaffolding, we dropped the ANSIBLE_GATHERING='explicit' environment variable on the operator's deployment. This causes a performance regression, as gathering facts can increase execution time by 30% for certain playbooks, and the gathered facts do not generally have any applicability to the operator use-case.

This also makes it so that when running roles directly, we disable fact gathering if ANSIBLE_GATHERING is set to explicit. This allows the user to avoid the aforementioned performance penalty without needing to wrap their role in a playbook.

Fixes #2358

Motivation for the change:
Performance regression due to an oversight when migrating to kubebuilder.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 22, 2020
@fabianvf fabianvf force-pushed the ansible-disable-default-gathering branch from 5028714 to 48e2c98 Compare September 24, 2020 16:39
@fabianvf fabianvf changed the title [WIP] Disable fact gathering by default Disable fact gathering by default Sep 25, 2020
@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 Sep 25, 2020
@geerlingguy
Copy link
Copy Markdown
Contributor

lgtm

@fabianvf fabianvf requested a review from asmacdo September 25, 2020 18:21
Eventually(managerContainerLogs, time.Minute, time.Second).Should(ContainSubstring(
"Ansible-runner exited successfully"))
Eventually(managerContainerLogs, time.Minute, time.Second).ShouldNot(ContainSubstring("failed=1"))
Eventually(managerContainerLogs, time.Minute, time.Second).ShouldNot(ContainSubstring("[Gathering Facts]"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh you are terrific 👍

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/approve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fact gathering still enabled

5 participants