Allow None value maximum_timeout for gitlab_runner

[colin93]

SUMMARY

Allow maximum_timeout for gitlab_runner to be set to None.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

gitlab_runner

ADDITIONAL INFORMATION

Currently trying to do this with null results in the below error.

fatal: [localhost]: FAILED! => {"changed": false, "msg": "argument 'maximum_timeout' is of type NoneType and we were unable to convert to int: \"None\" cannot be converted to an int"}

Values less than 600 are also rejected by the API, this PR converts 0 to None to allow for disabling timeout at the runner level as covered in example 2 of https://docs.gitlab.com/ci/runners/configure_runners/#how-maximum-job-timeout-works

- name: Create an instance-level runner
  community.general.gitlab_runner:
    api_url: https://gitlab.example.com/
    api_token: "{{ access_token }}"
    description: Docker Machine t1
    state: present
    active: true
    tag_list: ['docker']
    run_untagged: false
    locked: false
    maximum_timeout: 0
  register: runner # Register module output to run C(gitlab-runner register) command in another task

[ansibullbot]

cc @Lunik @SamyCoenen @Shaps @lgatellier @marwatk @metanovii @nejch @scodeman @sh0shin @suukit @waheedi @zanssa
click here for bot help

[russoz]

Hi @colin93 thanks for your contribution.

Some initial comments.

[colin93]

Hi @russoz , have updated changelog with suggestion.

As for the type there seems to be strict type matching, I can do some further testing to see if there's a better method to allow null for just the maximum_timeout field.
Otherwise as in below screenshot, without the changes (1st run) the module fails with type mismatch when using null vs with the changes (2nd run) it passes and correctly creates the runner.

[felixfontein]

The standard way of saying "no timeout" for timeout parameters that accept a number is to provide 0 or a negative number. The module can transform that into whatever the API accepts. Care must be taken for idempotency, since None in the module code usually means "don't change this parameter".

[colin93]

Have changed to instead convert 0 to None. The API itself only accepts positive values greater than 600 and None https://gitlab.com/gitlab-org/gitlab/-/blob/766d796437571129ded2bdcbdce3421547e776c6/app/models/ci/runner.rb#L272

[felixfontein]

I think the PR looks good now. I have a small suggestion for the docs though:

[felixfontein]

I'd mention when this got added:

Suggested change

	- The maximum time that a runner has to complete a specific job. Use V(0) to disable the timeout.
	- The maximum time that a runner has to complete a specific job.
	- Use V(0) to disable the timeout. This is available since community.general 12.1.0.

[russoz]

After changing the disable value to 0, I think the change in this line has become redundant, but I have not checked it thoroughly.

[colin93]

Since the value of maximum_timeout is already converted to None at this stage without this check a value of 0 would end up being skipped for updates.

[russoz]

And I am curious now, what does the module do when that parameter is indeed null (None)?

[felixfontein]

Ansible won't let you pass null here, since the parameter has a default value and is of type int.