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

3.4.0 Regression: dependency check does not completely honor the enable: false #3234

Closed
fcharlier opened this issue Sep 8, 2021 · 3 comments · Fixed by #3235 or #3236
Closed

3.4.0 Regression: dependency check does not completely honor the enable: false #3234

fcharlier opened this issue Sep 8, 2021 · 3 comments · Fixed by #3235 or #3236
Labels

Comments

@fcharlier
Copy link
Contributor

fcharlier commented Sep 8, 2021

Issue Type

  • Bug report

Molecule and Ansible details

>  .tox/py36/bin/molecule --version
molecule 3.4.0 using python 3.6
    ansible:2.9.25
    delegated:3.4.0 from molecule
    podman:1.0.1 from molecule_podman requiring collections: containers.podman>=1.7.0 ansible.posix>=1.3.0

Molecule installation method (one of):

  • pip

Ansible installation method (one of):

  • OS package

Desired Behavior

With molecule/default/molecule.yml containing

---
dependency:
  name: galaxy
  enabled: false

The dependency check should be print (output from molecule 3.3.4):

INFO     Running default > dependency
WARNING  Skipping, dependency is disabled.
WARNING  Skipping, dependency is disabled.
INFO     Running default > syntax

Actual Behaviour

Instead it tries to install the podman driver dependency:

INFO     Running default > dependency
INFO     Running ansible-galaxy collection install --force -v containers.podman:>=1.7.0
CRITICAL Collection 'containers.podman' not found in '['/home/[user]/.cache/ansible-lint/dbf977/collections', '/home/[user]/.ansible/collections', '/usr/share/ansible/collections']'
WARNING  An error occurred during the test sequence action: 'dependency'. Cleaning up.

I suppose it's due to #3192 which calls super.execute() before verifying if the dependency check is enabled or not in src/molecule/dependency/shell.py

@fcharlier fcharlier added the bug label Sep 8, 2021
@ssbarnea
Copy link
Member

ssbarnea commented Sep 8, 2021

@fcharlier I think you are right. Can you test locally if moving the super call to the else branch of the if is fixing the issue? If it works you can also make the PR for it.

The problem is important enough to make a bugfix release for it and I can help with that.

sf-project-io pushed a commit to redhat-cip/ansible-role-dci-retrieve-component that referenced this issue Sep 8, 2021
Exclude molecule 3.4 which does not honor completely the "enabled:
False" property for the dependency checker.

See ansible/molecule#3234

Change-Id: I94e9f39dd608699e8b15364f1233cf2dd5082320
@fcharlier
Copy link
Contributor Author

After moving the call to super.execute() after the if not self.enabled: check (see diffs at the end of the message), the dependency checker is skipped as expected.

INFO     Running default > dependency
WARNING  Skipping, dependency is disabled.
WARNING  Skipping, dependency is disabled.
INFO     Running default > syntax

The diffs

*** .tox/py36/lib/python3.6/site-packages/molecule/dependency/shell.py.old	2021-09-08 17:15:44.639945919 +0200
--- .tox/py36/lib/python3.6/site-packages/molecule/dependency/shell.py	2021-09-08 17:17:58.855439536 +0200
***************
*** 87,102 ****
      def bake(self) -> None:
          """Bake a ``shell`` command so it's ready to execute."""
          self._sh_command = BakedCommand(cmd=self.command, env=self.env)

      def execute(self):
-         super().execute()
          if not self.enabled:
              msg = "Skipping, dependency is disabled."
              LOG.warning(msg)
              return

          if self._sh_command is None:
              self.bake()

          self.execute_with_retries()

--- 87,103 ----
      def bake(self) -> None:
          """Bake a ``shell`` command so it's ready to execute."""
          self._sh_command = BakedCommand(cmd=self.command, env=self.env)

      def execute(self):
          if not self.enabled:
              msg = "Skipping, dependency is disabled."
              LOG.warning(msg)
              return

+         super().execute()
+
          if self._sh_command is None:
              self.bake()

          self.execute_with_retries()

*** .tox/py36/lib/python3.6/site-packages/molecule/dependency/ansible_galaxy/base.py.old	2021-09-08 17:20:17.016800665 +0200
--- .tox/py36/lib/python3.6/site-packages/molecule/dependency/ansible_galaxy/base.py	2021-09-08 17:20:20.980667576 +0200
***************
*** 111,126 ****
              cmd=[self.command, *self.COMMANDS, *util.dict2args(options), *verbose_flag],
              env=self.env,
          )

      def execute(self):
-         super().execute()
          if not self.enabled:
              msg = "Skipping, dependency is disabled."
              LOG.warning(msg)
              return

          if not self._has_requirements_file():
              msg = "Skipping, missing the requirements file."
              LOG.warning(msg)
              return

--- 111,126 ----
              cmd=[self.command, *self.COMMANDS, *util.dict2args(options), *verbose_flag],
              env=self.env,
          )

      def execute(self):
          if not self.enabled:
              msg = "Skipping, dependency is disabled."
              LOG.warning(msg)
              return

+         super().execute()
          if not self._has_requirements_file():
              msg = "Skipping, missing the requirements file."
              LOG.warning(msg)
              return

@fcharlier
Copy link
Contributor Author

@ssbarnea the #3235 commit fixes the issue only for the "shell" dependency checker, not for the "galaxy" checker.

The ansible_galaxy/base.py file requires the same change.

fcharlier added a commit to fcharlier/molecule that referenced this issue Sep 9, 2021
PR ansible#3235 was incomplete and fixed the issue only for shell dependency
checker. This PR fixes also for Galaxy dependency checker.

Fixes: ansible#3234
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants