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

Fix deploying to Windows machines #280

Merged
merged 4 commits into from
Sep 4, 2023
Merged

Conversation

alexdelifer
Copy link

It seems the current implementation breaks if there are no configured runners, this brings the window configuration in line with the other Unix configurations.

It seems to work on my side when testing just this included file.

@alexdelifer
Copy link
Author

I added some more commits that got deploying to a windows machine working again, it seems it had been broken for a good while.

Seems just like small stuff to me.

I only tested Shell executor runners on windows, I'll likely run some docker-windows executor jobs with this soon and can fix those if there's anything different.

@alexdelifer alexdelifer changed the title fixed "create list of configured runners" for Windows Fix deploying to Windows machines Aug 31, 2023
… automatically

useful for UI testing where the gitlab-runner.exe needs to be running as a process on the desktop in order to launch GUI processes properly.
@alexdelifer
Copy link
Author

I've successfully tested this role in my playbook that was previously running on v1.6.45 where the windows runner still worked.

I'll test the docker windows runners either this afternoon or tomorrow.

I've also added a little check for "gitlab_runner_windows_start_runner" which defaults to true like macos, so only people who toggle it are affected.
This greatly helps simplify setting up gitlab-runner "not as a service" but instead to be run from the desktop. When using UI testing frameworks like Squish for QT it's only possible to run the UI tests if the gitlab-runner is able to launch apps on the desktop, and when it's a service they don't open on the desktop.

@@ -17,8 +17,8 @@
registered_gitlab_runner_names: "{{ registered_gitlab_runner_names + [json_item['msg']] }}"
vars:
json_item: "{{ item | from_json }}"
loop: "{{ registered_runners_json_result.stderr.split('\n') }}"
loop: "{{ registered_runners_json_result.stderr_lines }}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this documentation https://docs.ansible.com/ansible/latest/collections/ansible/windows/win_command_module.html

There is no such field as stderr_lines.

Can you confirm that the documentation is wrong and the field is indeed there?

Copy link
Author

@alexdelifer alexdelifer Sep 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled that from here

loop: "{{ registered_runners_json_result.stderr_lines }}"

And it indeed works, when it was using the split('\n') thing there was a trailing newline/whitespace, which led to a blank {{ item }} and the task failing the when clause.

I think this might be one of those ansible "it works everywhere" kinda things.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 1000% sure, but this implies you can expect stderr_lines to always be present.
https://docs.ansible.com/ansible/latest/reference_appendices/common_return_values.html#stderr-lines

Probably just a case of the ansible community windows missing some docs on things they consider to be "expected"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome.

@alexdelifer
Copy link
Author

I can also confirm my docker-windows machines are successfully deployed and ready in gitlab with the changes from this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants