community.general icon indicating copy to clipboard operation
community.general copied to clipboard

Add Gitlab group runners support

Open lgatellier opened this issue 3 years ago • 13 comments

SUMMARY

This PR adds Gitlab group runners support to community.general.gitlab_runner module. This allow to register a new Gitlab Runner within group settings, its update & deletion as a non-admin user.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

gitlab_runner module

ADDITIONAL INFORMATION

This PR :

  • Adds a group string parameter to gitlab-runner module, similar to the project parameter, to specify the Gitlab group on which to register the runner
  • Fixes a bug on project & group runner creation/deletion, which was previously done using the project API, but needs to be done from root /runners API.
  • Adds a check to fail when both group and project and specified
- name: Register Gitlab Runner
  community.general.gitlab_runner:
    access_level: not_protected
    active: yes
    api_url: "{{ runner.gitlab_url }}"
    api_token: "{{ gitlab_api_token }}"
    description: "{{ inventory_hostname }}"
    locked: no
    group: "{{ runner.group }}"
    registration_token: "{{ runner.registration_token }}"
    state: present
    run_untagged: yes

lgatellier avatar Dec 22 '21 19:12 lgatellier

cc @Lunik @SamyCoenen @Shaps @dj-wasabi @marwatk @metanovii @scodeman @sh0shin @waheedi @zanssa click here for bot help

ansibullbot avatar Dec 22 '21 19:12 ansibullbot

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

plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/source_control/gitlab/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

click here for bot help

ansibullbot avatar Dec 22 '21 19:12 ansibullbot

CI failure is unrelated.

felixfontein avatar Dec 27 '21 18:12 felixfontein

@felixfontein I've done a few more tests, the PR is okay for me 😉

lgatellier avatar Dec 28 '21 15:12 lgatellier

Note : the unit tests I added make the module use the gitlab_group.runners.list() API, which does not exist before python-gitlab 2.3.0. CI pipeline installs python-gitlab 1.15.0.

@felixfontein What about deprecating the use of python-gitlab v1 which is not supported any more , since v2 has been release in January 2020, and latest release is v3.1.1 ? Or should I just mark this unit test as ignored (or maybe run this module tests with python-gitlab >= 2.3.0 ?) ?

@nejch your opinion would be useful here, too :-)

lgatellier avatar Feb 10 '22 13:02 lgatellier

@nejch your opinion would be useful here, too :-)

@lgatellier keep in mind that python-gitlab 2.0.0 dropped support for python < 3.6. Otherwise I agree :) some things like user/pass basic auth in python-gitlab v1 are now misleading since GitLab API v4 does not support it anyway (I see there's now an oauth ROPC workaround here in the modules).

For gitlab modules I guess they should really be delegated to the controller/localhost so supporting EOL python versions wouldn't be that important but maintainers might want to be a bit more conservative.

nejch avatar Feb 10 '22 22:02 nejch

We can deprecate python-gitlab 1.x.y, but we will still have to support it until the deprecation expires. For the tests, I would simply skip the tests that need a newer python-gitlab if that one isn't around.

felixfontein avatar Feb 12 '22 08:02 felixfontein

(In general see our policy in https://github.com/ansible-collections/community.general/issues/582#issue-645482173. Usually we do it like this: if we deprecate something in version x.y.0, we only actually remove support in version (x+2).0.0.)

felixfontein avatar Feb 12 '22 08:02 felixfontein

We can deprecate python-gitlab 1.x.y, but we will still have to support it until the deprecation expires. For the tests, I would simply skip the tests that need a newer python-gitlab if that one isn't around.

So there's no problem to deprecate Python < 3.6 (as nejch said in his comment) for a specific module ?

(In general see our policy in https://github.com/ansible-collections/community.general/issues/582#issue-645482173. Usually we do it like this: if we deprecate something in version x.y.0, we only actually remove support in version (x+2).0.0.)

Thanks a lot ! :-)

lgatellier avatar Feb 13 '22 19:02 lgatellier

@felixfontein I was thinking about adding a python-gitlab version number check when group parameter is used, and looked for similar cases in the collection. I found multiple occurrences of this kind of check (in circonus_annotation.py):

from ansible_collections.community.general.plugins.module_utils.version import LooseVersion

required_version = '2.0.0' if PY3 else '1.0.0'
if LooseVersion(requests.__version__) < LooseVersion(required_version):
    module.fail_json(msg="'requests' library version should be >= %s, found: %s." % (required_version, requests.__version__))

What do you think about creating a new method in module_utils.version, which could behave like that :

def assert_library_version(module, library_name, required_version, actual_version, reason=None):
    if LooseVersion(actual_version) < LooseVersion(required_version):
        if reason:
            reason = " Reason : %s" % reason
        module.fail_json(msg="'%s' library version should be >= %s, found: %s.%s" % (library_name, required_version, actual_version, reason))

It could be used like that :

required_version = '2.0.0' if PY3 else '1.0.0'
assert_library_version(module, "requests", required_version, requests.__version__)

And with a reason (in my case) :

if project:
    # ...
elif group: # L403 of gitlab_runner.py
    assert_library_version(module, "gitlab", "2.3.0", gitlab.__version__, "Use of 'group' parameter")

lgatellier avatar Feb 21 '22 19:02 lgatellier

cc @suukit click here for bot help

ansibullbot avatar Mar 14 '22 15:03 ansibullbot

What do you think about creating a new method in module_utils.version, which could behave like that :

works for me.
@felixfontein any thoughts about it?

markuman avatar Jun 03 '22 08:06 markuman

@lgatellier sorry, I missed your question completely!

I'm also fine with that idea. It's best to add that in a separate PR though.

felixfontein avatar Jun 03 '22 16:06 felixfontein

Any update on this one? There are conflicts now btw.

needs_info

felixfontein avatar Oct 25 '22 05:10 felixfontein

Please note that in #5461 the collection repository was restructured to remove the directory tree in plugins/modules/, and the corresponding tree in tests/unit/plugins/modules/. Your PR adds new files into this hierarchy. Please rebase with the current main branch and move your files directly into plugins/modules/. You also can remove the changes to meta/runtime.yml, these are not needed anymore (the corresponding section was removed from CONTRIBUTING.md), and make sure to adjust the new entries in .github/BOTMETA.yml as well.

felixfontein avatar Nov 03 '22 06:11 felixfontein

@lgatellier This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

ansibullbot avatar Nov 27 '22 07:11 ansibullbot

Hi @felixfontein,

I'll rebase/rewrite this PR to match newrepo structure before removing Draft status. :wink:

lgatellier avatar Nov 27 '22 11:11 lgatellier

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

plugins/modules/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

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

plugins/modules/gitlab_runner.py:0:0: parameter-type-not-in-doc: Argument 'group' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/gitlab_runner.py:0:0: undocumented-parameter: Argument 'group' is listed in the argument_spec, but not documented in the module documentation

click here for bot help

ansibullbot avatar Dec 06 '22 19:12 ansibullbot

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

github-actions[bot] avatar Dec 06 '22 19:12 github-actions[bot]

@felixfontein Just back from some busy months, I rebased on main branch, and run some more tests with instance/group/project runners on a GitLab 15.8 self-hosted instance.

Thus I think this PR is finally ready to get a review :wink:

lgatellier avatar Mar 11 '23 19:03 lgatellier

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/f3be0076af3c7869edd9f3360bf6b4bc25cd93cb/pr-3935

Backported as https://github.com/ansible-collections/community.general/pull/6234

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

patchback[bot] avatar Mar 25 '23 07:03 patchback[bot]

@lgatellier thanks a lot for your contribution! @nejch @markuman thanks for your reviews!

felixfontein avatar Mar 25 '23 07:03 felixfontein