'knife node run_list add' should take a list of RunListItems

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Unknown Unknown
  • Resolution: Fixed
  • Affects Version/s: 0.10.8
  • Fix Version/s: 0.10.10
  • Component/s: Knife
  • Labels:
    None

Description

'knife node run_list add' expects a single entry which it adds to the run_list. You should be able to provide a list of items. Currently if you try to provide multiple items you hit CHEF-1398.

Further, 'knife bootstrap' and 'knife ec2 server create' have been known to take a run list in different formatting. We should standardize on a single mixin for these. The knife ec2 plugin will have to wait until we ship a version of core shef that includes the mixin before switching.

Issue Links

relates to

Bug - A problem which impairs or prevents the functions of the product. CHEF-1398 Run List Item regex is too lenient, creating bizarre invalid item names

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Juanje Ojeda added a comment -

I create a pull-request with a possible solution to this bug at https://github.com/opscode/chef/pull/244

I did create a test and I tried by hand in my environment and is working fine. i also changed ENTRY by ENTRIES in the help description to avoid confusion.

I hope this fix the issue, it was annoying me to.

Show
Juanje Ojeda added a comment - I create a pull-request with a possible solution to this bug at https://github.com/opscode/chef/pull/244 I did create a test and I tried by hand in my environment and is working fine. i also changed ENTRY by ENTRIES in the help description to avoid confusion. I hope this fix the issue, it was annoying me to.
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

One idea was parsing all of @name_args by removing the node name, e.g.

ruby-1.9.2-p290 :001 > %w{foo bar baz qux quux}[1..-1]
 => ["bar", "baz", "qux", "quux"]

However, knife ec2 server create -r takes a comma separated list so we want to be prepared for that.

Can we add more test cases here to make sure we've got good coverage?

# with a space
@knife.name_args = [ "adam", "role[monkey], role[duck]" ]
# an extraneous comma
@knife.name_args = [ "adam", "role[monkey]," ]
Show
Bryan McLellan [Opscode] added a comment - One idea was parsing all of @name_args by removing the node name, e.g. ruby-1.9.2-p290 :001 > %w{foo bar baz qux quux}[1..-1] => [ "bar" , "baz" , "qux" , "quux" ] However, knife ec2 server create -r takes a comma separated list so we want to be prepared for that. Can we add more test cases here to make sure we've got good coverage? # with a space @knife.name_args = [ "adam" , "role[monkey], role[duck]" ] # an extraneous comma @knife.name_args = [ "adam" , "role[monkey]," ]
Hide
Permalink
Juanje Ojeda added a comment -

I've added those test cases and remove the extra spaces (in case they were any) that wasn't working. It's at the pull-request.

But I wasn't sure about doing something on the parsing @name_args thing. I'm not sure what do you want to do.

I think we could support passing items with or without comas, but not mixed. I mean:

# with comas
knife node run_list add adam role[monkey], role[duck],recipe[bird::fly]

# without comas
knife node run_list add adam role[monkey] role[duck] recipe[bird::fly]

But I don't think mixing both make much sense. Do you?

Show
Juanje Ojeda added a comment - I've added those test cases and remove the extra spaces (in case they were any) that wasn't working. It's at the pull-request. But I wasn't sure about doing something on the parsing @name_args thing . I'm not sure what do you want to do. I think we could support passing items with or without comas, but not mixed. I mean: # with comas knife node run_list add adam role[monkey], role[duck],recipe[bird::fly] # without comas knife node run_list add adam role[monkey] role[duck] recipe[bird::fly] But I don't think mixing both make much sense. Do you?
Hide
Permalink
Juanje Ojeda added a comment -

Well, I added and implemented the test cases and the mixed one. I created a single commit in case you see it good enough to be merged. It's at the pull request.

No it checks (and pass) the follow cases:

# with comas
@knife.name_args = [ "adam", "role[monkey],role[duck]" ]

# with comas and a space
@knife.name_args = [ "adam", "role[monkey], role[duck]" ]

# with no comas ( knife node run_list add adam role[monkey] role[duck] )
@knife.name_args = [ "adam", "role[monkey]", "role[duck]" ]

# mixed arguments and comas ( knife node run_list add adam role[monkey] role[duck],recipe[bird::fly] )
@knife.name_args = [ "adam", "role[monkey]", "role[duck],recipe[bird::fly]" ]

# an extraneous comma
@knife.name_args = [ "adam", "role[monkey]," ]

Any other suggestiongs? Does it fit with what you had in mind?
Thanks for your feedback and help

Show
Juanje Ojeda added a comment - Well, I added and implemented the test cases and the mixed one. I created a single commit in case you see it good enough to be merged. It's at the pull request. No it checks (and pass) the follow cases: # with comas @knife.name_args = [ "adam" , "role[monkey],role[duck]" ] # with comas and a space @knife.name_args = [ "adam" , "role[monkey], role[duck]" ] # with no comas ( knife node run_list add adam role[monkey] role[duck] ) @knife.name_args = [ "adam" , "role[monkey]" , "role[duck]" ] # mixed arguments and comas ( knife node run_list add adam role[monkey] role[duck],recipe[bird::fly] ) @knife.name_args = [ "adam" , "role[monkey]" , "role[duck],recipe[bird::fly]" ] # an extraneous comma @knife.name_args = [ "adam" , "role[monkey]," ] Any other suggestiongs? Does it fit with what you had in mind? Thanks for your feedback and help
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Marked to be merged.

Show
Bryan McLellan [Opscode] added a comment - Marked to be merged.
Hide
Permalink
Bryan McLellan [Opscode] added a comment -

Merged to master. Thanks again Juanje. These are going to help a lot of people.

Show
Bryan McLellan [Opscode] added a comment - Merged to master. Thanks again Juanje. These are going to help a lot of people.

People

  • Assignee:
    Bryan McLellan [Opscode]
    Reporter:
    Bryan McLellan [Opscode]
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: