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

[vars_plugins] gather vars once per unique directory and entities #58424

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@das7pad
Copy link
Contributor

commented Jun 26, 2019

SUMMARY

The inventory may sit next to the playbook. Inventory directory and
playbook directory overlap in this case. Causing a duplicate run.

Depending on the configured loading sequence a given directory may be
loaded as an inventory directory or as a playbook directory.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vars_plugins

ADDITIONAL INFORMATION

Reduces the invocation count in #47249 from 36 to 18:

36 = 3 nodes * (gather facts + ls-task) * (all-group + nodes-group + host) * (inventory-dir + playbook-dir)

Playbook directory and inventory directory are the same, we can drop the duplicate call.


Output of the new integration test without the patch

$ REPO=$PWD; cd ./test/integration/targets/plugin_vars_once && ./runme.sh 2>&1 | sed "s#${REPO}#REPO#g"; cd "$REPO"
CONFIG: configs/default.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
2d1
< REPO/test/integration/targets/plugin_vars_once [all]
4,5d2
< REPO/test/integration/targets/plugin_vars_once [testgroup]
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/default.cfg INVENTORY: REPO/test/integration/inventory
CONFIG: configs/no_inventory.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
4d3
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/no_inventory.cfg INVENTORY: REPO/test/integration/inventory
CONFIG: configs/no_play.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
4d3
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/no_play.cfg INVENTORY: REPO/test/integration/inventory
CONFIG: configs/no_plugins.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
2d1
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/no_plugins.cfg INVENTORY: REPO/test/integration/inventory
CONFIG: configs/reverse.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
2d1
< REPO/test/integration/targets/plugin_vars_once [testgroup]
4,5d2
< REPO/test/integration/targets/plugin_vars_once [all]
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/reverse.cfg INVENTORY: REPO/test/integration/inventory
CONFIG: configs/reverse_no_inventory.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
4d3
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/reverse_no_inventory.cfg INVENTORY: REPO/test/integration/inventory
CONFIG: configs/reverse_no_play.cfg INVENTORY: REPO/test/integration/targets/plugin_vars_once/inventory
4d3
< REPO/test/integration/targets/plugin_vars_once [testhost]
---
Full log:
REPO/test/integration/targets/plugin_vars_once [testgroup]
REPO/test/integration/targets/plugin_vars_once [all]
REPO/test/integration/targets/plugin_vars_once [testhost]
REPO/test/integration/targets/plugin_vars_once [testhost]
---
CONFIG: configs/reverse_no_play.cfg INVENTORY: REPO/test/integration/inventory
[vars_plugins] gather vars once per unique directory and entities
The inventory may sit next to the playbook. Inventory directory and
 playbook directory overlap in this case. Causing a duplicate run.

Depending on the configured loading sequence a given directory may be
 loaded as an inventory directory or as a playbook directory.

Signed-off-by: Jakob Ackermann <das7pad@outlook.com>
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

The test ansible-test sanity --test integration-aliases [explain] failed with 1 error:

test/integration/targets/plugin_vars_once/aliases:0:0: missing alias `shippable/posix/group[1-4]` or `unsupported`

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/vars/manager.py:235:13: E303 too many blank lines (2)
test/integration/targets/plugin_vars_once/vars_plugins/logger.py:3:1: E302 expected 2 blank lines, found 1

The test ansible-test sanity --test shellcheck [explain] failed with 15 errors:

test/integration/targets/plugin_vars_once/runme.sh:4:11: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:9:23: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:12:53: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:18:9: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:26:8: SC2155 Declare and assign separately to avoid masking return values.
test/integration/targets/plugin_vars_once/runme.sh:30:33: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:34:18: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:34:47: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:41:15: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:41:55: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:46:23: SC2046 Quote this to prevent word splitting.
test/integration/targets/plugin_vars_once/runme.sh:47:18: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:47:47: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:54:15: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/plugin_vars_once/runme.sh:54:55: SC2086 Double quote to prevent globbing and word splitting.

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Jun 26, 2019

das7pad added some commits Jun 26, 2019

[misc] fix lint errors
Signed-off-by: Jakob Ackermann <das7pad@outlook.com>
[misc] fix the integration test for the vars_plugins
- move output to stderr
- fix encoding error: strip repository root from paths
- forward runme.sh arguments to ansible

Signed-off-by: Jakob Ackermann <das7pad@outlook.com>
[misc] fix the integration test for OSs with an old basename(1) version
Signed-off-by: Jakob Ackermann <das7pad@outlook.com>
[misc] fix the integration test for OSs without truncate(1)
Signed-off-by: Jakob Ackermann <das7pad@outlook.com>
[misc] bump CI
Signed-off-by: Jakob Ackermann <das7pad@outlook.com>

@ansibot ansibot removed the ci_verified label Jun 26, 2019

@das7pad

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

Seems to be a github/CI issue here. I pushed to a different branch, waited a few minutes and queried the .patch github-URL of the commit from multiple locations around the globe. Never the less the CI can not find the commit in the first couple runs.

Pushing new commits will likely fail again. A manually triggered rebuild may succeed?

@mattclay

This comment has been minimized.

Copy link
Member

commented Jun 27, 2019

Closing and re-opening to see if that fixes the missing commit issue.

@mattclay mattclay closed this Jun 27, 2019

@mattclay mattclay reopened this Jun 27, 2019

@das7pad

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

Is it possible to rerun the single 'unstable' test-run?

@mattclay

This comment has been minimized.

Copy link
Member

commented Jun 27, 2019

@das7pad Yes, restarted.

@jillr jillr requested a review from bcoca Jun 27, 2019

@jillr jillr removed the needs_triage label Jun 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.