Skip to content

Add Ansible diff mode #98

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

Merged
ekohl merged 1 commit into from
Jun 24, 2025
Merged

Add Ansible diff mode #98

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/smart_proxy_ansible/runner/ansible_runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def initialize(input, suspended_action:, id: nil)
@rex_command = action_input[:remote_execution_command]
@check_mode = action_input[:check_mode]
@job_check_mode = action_input[:job_check_mode]
@diff_mode = action_input[:diff_mode]
Copy link
Member

@ekohl ekohl Jun 21, 2025

Choose a reason for hiding this comment

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

Looking at the naming: #100 chose job_diff_mode. For check_mode there's both check_mode and job_check_mode. d67b3d2 added that.

Should we support both too here?

Copy link
Contributor Author

@rpluem-vf rpluem-vf Jun 23, 2025

Choose a reason for hiding this comment

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

We could, but this would require changes to theforeman/foreman_ansible#749 as well similar to theforeman/foreman_ansible@76a53e3 I guess. Let me know what you prefer.

Copy link
Contributor Author

@rpluem-vf rpluem-vf Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We could, but this would require changes to theforeman/foreman_ansible#749 as well similar to theforeman/foreman_ansible@76a53e3 I guess. Let me know what you prefer.

Looking at theforeman/foreman_ansible#764 and #100 I think no changes are needed here and we just need to merge these and mine. There are some text conflicts but these should be easy to fix in a rebase of whatever PR is merged later.

Copy link
Member

@ekohl ekohl Jun 23, 2025

Choose a reason for hiding this comment

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

@bnerickson has submitted theforeman/foreman_ansible#764. I wouldn't mind merging this PR together with your foreman_ansible PR. @bnerickson would you be OK to rebase your work on top of this?

Copy link
Contributor

@bnerickson bnerickson Jun 23, 2025

Choose a reason for hiding this comment

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

I have no problem rebasing my work on top of this PR.

@tags = action_input[:tags]
@tags_flag = action_input[:tags_flag]
@passphrase = action_input['secrets']['key_passphrase']
Expand Down Expand Up @@ -214,7 +215,7 @@ def start_ansible_runner
end

def cmdline
cmd_args = [tags_cmd, check_cmd].reject(&:empty?)
cmd_args = [tags_cmd, check_cmd, diff_cmd].reject(&:empty?)
return nil unless cmd_args.any?
cmd_args.join(' ')
end
Expand All @@ -232,6 +233,10 @@ def check_cmd
end
end

def diff_cmd
diff_mode? ? '"--diff"' : ''
end

def verbosity
'-' + 'v' * @verbosity_level.to_i
end
Expand All @@ -248,6 +253,10 @@ def job_check_mode?
@job_check_mode == true
end

def diff_mode?
@diff_mode == true
end

def prepare_directory_structure
inner = %w[inventory project env].map { |part| File.join(@root, part) }
([@root] + inner).each do |path|
Expand Down