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

Set packer_http_addr of extra-vars as an empty string when it's nil #174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

126ium
Copy link

@126ium 126ium commented Sep 13, 2023

Fixes #173. This fix involves examining the generatedData to determine if the PackerHTTPAddr is nil. If the PackerHTTPAddr is nil, set it as an empty string. This modification ensures the proper handling of the packer_http_addr variable within the ansible-local provisioner.

Test

  1. Navigate to the source directory of Ansible-local.
  2. Run the test cases as follows:
[ansible-local]$ go test
2023/09/13 14:38:13 ui: Provisioning with Ansible...
2023/09/13 14:38:13 ui: Creating Ansible staging directory...
2023/09/13 14:38:13 ui: Creating directory: /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5
2023/09/13 14:38:13 ui: Uploading playbook file: /tmp/2605838767
2023/09/13 14:38:13 ui: Creating directory: /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5/tmp
2023/09/13 14:38:13 ui: Uploading playbook file: /tmp/239453821
2023/09/13 14:38:13 ui: Creating directory: /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5/tmp
2023/09/13 14:38:13 ui: Uploading playbook file: /tmp/1305372664
2023/09/13 14:38:13 ui: Creating directory: /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5/tmp
2023/09/13 14:38:13 ui: Uploading inventory file...
2023/09/13 14:38:13 ui: Executing Ansible: cd /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5 &&  ANSIBLE_FORCE_COLOR=1 PYTHONUNBUFFERED=1 ansible-playbook /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5/tmp/2605838767 --extra-vars "packer_build_name= packer_builder_type= packer_http_addr= -o IdentitiesOnly=yes"  -c local -i /tmp/packer-provisioner-ansible-local/65013cb5-9e19-0c3d-9ee7-ca232dba2af5/packer-provisioner-ansible-local1541997294

packer_http_addr should be an empty string, not %!s(<nil>).

Closes #173

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Hi @126ium,

Thanks for the PR!

I just left one comment regarding the usage of the option, I think that if it is unset, it is probably better to not include it in the extra args instead of leaving it empty, but that could be a valid use though, I'll let you tell me what you think.

Aside from that, it looks good to me, we can merge this once we've reached a decision on this.

Thanks again!

@@ -549,8 +549,12 @@ func (p *Provisioner) invokeGalaxyCommand(args []string, ui packersdk.Ui, comm p
func (p *Provisioner) executeAnsible(ui packersdk.Ui, comm packersdk.Communicator) error {
inventory := filepath.ToSlash(filepath.Join(p.config.StagingDir, filepath.Base(p.config.InventoryFile)))

packerHTTPAddr := p.generatedData["PackerHTTPAddr"]
if packerHTTPAddr == nil {
packerHTTPAddr = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest maybe not adding the value to the extra-vars instead?
Unless this is used in the target, but then I would assume that it being empty would probably yield an error, so it may or may not be a good idea, what do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your reply and suggestion. Your idea is better. The ansible-playbook already provides a good handler for empty/default extra-vars, so I believe there will be no potential error. It's unnecessary to add them explicitly when they are empty.

I'm testing new codes with my module. Then I'll update this patch once I get any updates.

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.

ansible-local provisioner cannot set packer_http_addr properly when the variable is not implemented
2 participants