redhat-cop / katacoda-scenarios

Apache License 2.0
1 stars 8 forks source link

Fix indention level in requirements.yml #10

Closed jamcole closed 5 years ago

etsauer commented 5 years ago

@jamcole I'm not sure I agree that these changes are necessary. If you look in some of our examples, none of our requirements.yml files are indented this way, and we typically just specify inventory directory, not the hosts file.

Are there issues you are experiencing?

jamcole commented 5 years ago

Yeah, I talked to Patrick about this and it was unusual considering it has worked before for you... I don't know if a library or ansible version change somewhere changed this behavior, but if you run through it now without changing the indention you get this:

> EOM
$ cat <<EOM >requirements.yml
> - name: openshift-applier
>     scm: git
>     src: https://github.com/redhat-cop/openshift-applier
>     version: v2.0.2
> EOM
$ cat requirements.yml
- name: openshift-applier
    scm: git
    src: https://github.com/redhat-cop/openshift-applier
    version: v2.0.2
$ ansible-galaxy install -r requirements.yml -p roles
ERROR! Unable to load data from the requirements file: requirements.yml
etsauer commented 5 years ago

hmm.. @oybed @logandonley what do you think about this?

etsauer commented 5 years ago

@jamcole what ansible version is shown in the scenario?

jamcole commented 5 years ago

@etsauer

$ ansible-galaxy --version
ansible-galaxy 2.7.5
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible-galaxy
  python version = 2.7.5 (default, Nov  6 2016, 00:28:07) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)]
jamcole commented 5 years ago

@etsauer The requirements.yml generated in the secrets-with-openshift-applier scenerio has the indention level as specified in this PR and works properly: https://github.com/redhat-cop/katacoda-scenarios/edit/master/secrets-with-openshift-applier/step6.md

oybed commented 5 years ago

@jamcole @etsauer the indentation in the implementation as-is is wrong, so the fix/change in this PR is good.

However, I agree with Eric's second point and that we should not specify the hosts file as part of the inventory. This typically causes issues as it will "ignore" the rest of the inventory directory if a specific file is specified.

jamcole commented 5 years ago

No problem removing hosts filename at all; I haven't encountered the ignoring of other things in the inventory directory while using it, but I was mistaken about it being required (thought it was a typo).

PR #12 and #13 are also necessary for this scenario to run correctly.