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 build name ConfigTemplate processing [GH-858] #1402

Merged
merged 1 commit into from
Nov 26, 2014

Conversation

jasonberanek
Copy link
Collaborator

Addresses issue with build name ConfigTemplate processing causes errors with -only and -except command line flags, and with post-processor and provisioner only and except attributes.

The approach allows backwards compatibility with anyone using a build name template string (e.g., {{user "foo"}}) in -only or -except command line calls.

Also, added tests to verify behavior in the command line, provisioner definition, and post-processor definition.

I'd appreciate a head-nod that this looks good from another committer.

/cc @strcrzy

@strcrzy
Copy link
Contributor

strcrzy commented Aug 12, 2014

i will gladly take a look at this asap

@nchammas
Copy link
Contributor

What's the status of this PR? Still waiting for review?

I'd help review it, but I'm brand new to Packer and Go.

@nchammas
Copy link
Contributor

Alright, check out this Packer template:

{
  "variables": {
    "aws_access_key": "{{env `AWS_ACCESS_KEY_ID`}}",
    "aws_secret_key": "{{env `AWS_SECRET_ACCESS_KEY`}}",
    "spark_version": "1.1.0"
  },

  "builders": [
    {
      "name": "spark:base:us-east-1:ebs:hvm",
      "type": "amazon-ebs",
      "access_key": "{{user `aws_access_key`}}",
      "secret_key": "{{user `aws_secret_key`}}",
      "region": "us-east-1",
      "source_ami": "ami-08842d60",
      "instance_type": "m3.medium",
      "ssh_username": "ec2-user",
      "ami_name": "Spark Base HVM {{timestamp}}",
      "ami_description": "Spark Base HVM AMI",
      "ami_groups": ["all"],
      "ami_virtualization_type": "hvm"
    },
    {
      "name": "spark:{{user `spark_version`}}:us-east-1:ebs:hvm",
      "type": "amazon-ebs",
      "access_key": "{{user `aws_access_key`}}",
      "secret_key": "{{user `aws_secret_key`}}",
      "region": "us-east-1",
      "source_ami": "ami-08842d60",
      "instance_type": "m3.medium",
      "ssh_username": "ec2-user",
      "ami_name": "Spark {{user `spark_version`}} HVM {{timestamp}}",
      "ami_description": "Spark {{user `spark_version`}} HVM AMI",
      "ami_groups": ["all"],
      "ami_virtualization_type": "hvm"
    }
  ],

  "provisioners": [
    {
      "type": "shell",
      "execute_command": "chmod +x {{ .Path }}; {{ .Vars }} sudo -E sh -x '{{ .Path }}'",
      "inline": [
        "echo 'This will obviously work.'"
      ],
      "pause_before": "5s"
    },
    {
      "type": "shell",
      "only": [
        "spark:{{user `spark_version`}}:us-east-1:ebs:hvm"
      ],
      "execute_command": "chmod +x {{ .Path }}; {{ .Vars }} sudo -E sh -x '{{ .Path }}'",
      "inline": [
        "echo 'How about this? ༼⁰o⁰;༽'"
      ]
    }
  ]
}

Note that one builder has a static name, while the other has a name that depends on a user-defined variable.

I tried master at 7e4dfcd283 on this template and I didn't get any ༼⁰o⁰;༽. I then applied this patch and retried, and sure enough, I got ༼⁰o⁰;༽ all up in my face.

From my little test, this patch gets a 👍 from me.

@nchammas
Copy link
Contributor

nchammas commented Nov 8, 2014

@jasonberanek Do you know if this PR will be reviewed and merged in in time for the 0.7.3 release (or whatever the next release is)? The issue it addresses, #858, is a bug in the basic usability of template variables.

@jasonberanek
Copy link
Collaborator Author

@nchammas Thanks for testing the patch, and verify it works as expected.

I apologize for not responding sooner, been pulled away from working Packer things. I'll do my best to get this in the pipe for the next release. I still want at least another committer to ok before I push, so I'll reach out to them to get another set of eyes on it.

@nchammas
Copy link
Contributor

nchammas commented Nov 8, 2014

Okie doke. Thanks for the update!

@thewwr
Copy link

thewwr commented Nov 16, 2014

Could you please merge this PR? It would be important and appreciated. Thank you!

@sethvargo
Copy link
Contributor

@jasonberanek this looks good to me. Great work!

sethvargo added a commit that referenced this pull request Nov 26, 2014
fix build name ConfigTemplate processing [GH-858]
@sethvargo sethvargo merged commit 347f02a into hashicorp:master Nov 26, 2014
@ghost ghost locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants