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.

Sign up for GitHub

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

Jump to bottom

Fixes #36495 - Extend Windows templates for Puppet and Ansible #9732

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Fixes #36495 - Extend Windows templates for Puppet and Ansible #9732

merged 1 commit into from
Jul 30, 2024

Conversation

nadjaheitmann
Copy link
Contributor

No description provided.

@nadjaheitmann nadjaheitmann force-pushed the 36495_extend_windows_support branch 4 times, most recently from 84db3dc to e589d01 Compare June 9, 2023 11:30
@nadjaheitmann nadjaheitmann changed the title Fixes #36945 - Extend Windows templates for Puppet and Ansible Fixes #36495 - Extend Windows templates for Puppet and Ansible Jun 9, 2023
@nadjaheitmann nadjaheitmann force-pushed the 36495_extend_windows_support branch 4 times, most recently from 1cf378d to fdee96a Compare June 29, 2023 12:19
@nadjaheitmann
Copy link
Contributor Author

@ekohl Anything else that should be improved?

@nadjaheitmann nadjaheitmann force-pushed the 36495_extend_windows_support branch 2 times, most recently from cf9b139 to 46f0184 Compare October 26, 2023 12:02
@nadjaheitmann
Copy link
Contributor Author

@ekohl @sbernhard Can we move forward here?

ekohl
ekohl requested changes Oct 26, 2023
View reviewed changes
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
ekohl
ekohl previously requested changes Oct 27, 2023
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Please consistently use -%>, at least for newly added blocks.

app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
@nadjaheitmann nadjaheitmann force-pushed the 36495_extend_windows_support branch 2 times, most recently from c0a5564 to 4b939a4 Compare April 22, 2024 11:57
@nadjaheitmann nadjaheitmann mentioned this pull request May 3, 2024
Closed
ekohl
ekohl reviewed Jun 21, 2024
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Overall I think this looks pretty good and many notes are more nit picks.

app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/finish/windows_default_finish.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/provision/windows_default_provisioning.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/provision/windows_default_provisioning.erb Outdated Show resolved Hide resolved
app/views/unattended/provisioning_templates/script/windows_default_script.erb Outdated Show resolved Hide resolved
...n/renderer/snapshots/ProvisioningTemplate/script/Windows_peSetup.cmd.windows10_dhcp.snap.txt Outdated
@echo off
set WGET=wget64.exe

zerombr
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's using a Red Hat partitioning table. That's probably a bug in the factory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a partition table for windows. I am absolutely not sure if I added a correct format, though.

app/services/foreman/template_snapshot_service.rb Show resolved Hide resolved
nadjaheitmann
nadjaheitmann commented Jul 2, 2024
View reviewed changes
...enderer/snapshots/ProvisioningTemplate/finish/Windows_default_finish.windows10_dhcp.snap.txt Outdated
echo Downloading Puppet installer
wget "" -O C:\puppet-agent-x64-latest.msi
echo Installing Puppet
start /w "" msiexec /qn /i C:\puppet-agent-x64-latest.msi PUPPET_AGENT_STARTUP_MODE=Manual PUPPET_SERVER= PUPPET_CA_SERVER= PUPPET_AGENT_ACCOUNT_DOMAIN=snap.example.com PUPPET_AGENT_ACCOUNT_USER=administrator PUPPET_AGENT_ACCOUNT_PASSWORD=""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What looks funny to me is that PUPPET_SERVER= and PUPPET_CA_SERVER= are set without any values here. @ekohl Is this okay for the test or is it possible to set default values for host_puppet_server and host_puppet_ca_server?

@nadjaheitmann nadjaheitmann force-pushed the 36495_extend_windows_support branch 2 times, most recently from 10c3f6a to bf8664d Compare July 3, 2024 10:10
@brimioulle @ekohl @nadjaheitmann
Fixes #36495 - Extend Windows templates for Puppet and Ansible
f2e2718 
Co-authored-by: Fabrice Brimioulle <[email protected]>
Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
@nadjaheitmann
Copy link
Contributor Author

Overall I think this looks pretty good and many notes are more nit picks.

Thanks again @ekohl for the review. I addressed all the issues you mentioned. Do you mind having another look?

@nadjaheitmann nadjaheitmann requested a review from ekohl July 10, 2024 06:05
@nadjaheitmann
Copy link
Contributor Author

@ekohl More changes required? :)

@stejskalleos stejskalleos dismissed ekohl’s stale review July 30, 2024 11:28

Changes have been implemented

@stejskalleos stejskalleos merged commit 0d70063 into theforeman:develop Jul 30, 2024
53 checks passed
@stejskalleos
Copy link
Contributor

Thanks @nadjaheitmann !

@nadjaheitmann nadjaheitmann deleted the 36495_extend_windows_support branch July 30, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtruestedt jtruestedt jtruestedt left review comments

@stejskalleos stejskalleos stejskalleos approved these changes

@sbernhard sbernhard sbernhard left review comments

@ekohl ekohl Awaiting requested review from ekohl

Assignees

@sbernhard sbernhard

Labels
Templates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nadjaheitmann @stejskalleos @ekohl @sbernhard @jtruestedt @theforeman-bot