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

[MSYS-765]Feature chef-tag & aws-tag option #520

Merged

Conversation

dheerajd-msys
Copy link
Contributor

@dheerajd-msys dheerajd-msys commented Feb 12, 2018

Description

--aws-tag and --chef-tag options are added for tagging EC2 instance in AWS and Chef separately.

Deprecations

--tags option is now deprecated and we are going to use --aws-tag for EC2 tags e.g. --aws-tag <key1=value1>
--tag-node-in-chef is deprecated and we are going to use --chef-tag for Chef Tag e.g. --chef-tag <myTag>

Issues Resolved

Fixes #509

Signed-off-by: dheerajd-msys <dheeraj.dubey@msystechnologies.com>
Signed-off-by: dheerajd-msys <dheeraj.dubey@msystechnologies.com>
@dheerajd-msys dheerajd-msys changed the title [MSYS-765][WIP]Added chef-tag option and spec [MSYS-765][WIP]Feature chef-tag & aws-tag option Feb 13, 2018
@dheerajd-msys dheerajd-msys changed the title [MSYS-765][WIP]Feature chef-tag & aws-tag option [MSYS-765]Feature chef-tag & aws-tag option Feb 14, 2018
:boolean => true,
:default => false

option :instance_initiated_shutdown_behavior,
:long => "--instance-initiated-shutdown-behavior SHUTDOWN_BEHAVIOR",
:description => "Indicates whether an instance stops or terminates when you initiate shutdown from the instance. Possible values are 'stop' and 'terminate', default is 'stop'."

option :chef_tag,
:long => "--chef-tag CHEF_TAG",
:description => "The Chef tag for this server; Use the --chef-tag option multiple times when specifying multiple tags e.g. --chef-tag tag1 --chef-tag tag2.",

Choose a reason for hiding this comment

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

"Use to tag the node in chef server. Provide --chef-tag option multiple times when specifying multiple tags for node e.g. --chef-tag tag1 --chef-tag tag2.

@@ -817,6 +843,7 @@ def bootstrap_for_windows_node(server, fqdn)
bootstrap.config[:install_as_service] = locate_config_value(:install_as_service)
bootstrap.config[:session_timeout] = locate_config_value(:session_timeout)
bootstrap.config[:tags] = config[:tags] if locate_config_value(:tag_node_in_chef)
bootstrap.config[:tags] = config[:chef_tag] if locate_config_value(:chef_tag)
Copy link

@Vasu1105 Vasu1105 Feb 16, 2018

Choose a reason for hiding this comment

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

This looks to be a repetition of code in windows and linux block. Move this to bootstrap_common_params

@@ -477,15 +478,15 @@
end

it "sets the Name tag to the specified name when given --tags Name=NAME" do
knife_ec2_create.config[:tags] = ["Name=bobcat"]
knife_ec2_create.config[:aws_tag] = ["Name=bobcat"]

Choose a reason for hiding this comment

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

it block doesn't match the actual test case

expect(ec2_connection.tags).to receive(:create).with(:key => "Name",
:value => "bobcat",
:resource_id => new_ec2_server.id)
knife_ec2_create.run
end

it "sets arbitrary tags" do
knife_ec2_create.config[:tags] = ["foo=bar"]
knife_ec2_create.config[:aws_tag] = ["foo=bar"]

Choose a reason for hiding this comment

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

change the description as aws tags

@Vasu1105
Copy link

Vasu1105 commented Feb 16, 2018

@dheerajd-msys please add specs for deprecated options
Also add the specs for --aws-tag multiple values handling

@dheerajd-msys
Copy link
Contributor Author

Thanks @Vasu1105, I will get through this.

Signed-off-by: dheerajd-msys <dheeraj.dubey@msystechnologies.com>
Signed-off-by: dheerajd-msys <dheeraj.dubey@msystechnologies.com>
@dheerajd-msys dheerajd-msys force-pushed the dh/feature_aws_and_chef_tag_separate branch from c768459 to 2b8ec76 Compare February 23, 2018 07:34
:boolean => true,
:default => false

option :instance_initiated_shutdown_behavior,
:long => "--instance-initiated-shutdown-behavior SHUTDOWN_BEHAVIOR",
:description => "Indicates whether an instance stops or terminates when you initiate shutdown from the instance. Possible values are 'stop' and 'terminate', default is 'stop'."

option :chef_tag,
:long => "--chef-tag CHEF_TAG",
:description => "Used to tag the node in chef server; Provide --chef-tag option multiple times when specifying multiple tags e.g. --chef-tag tag1 --chef-tag tag2.",

Choose a reason for hiding this comment

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

It should be Use to not Used to

let(:ec2_server_create) { Chef::Knife::Ec2ServerCreate.new(['--tag-node-in-chef'])}
it 'raises deprecated warning "[DEPRECATED] --tag-node-in-chef option is deprecated. Use --chef-tag option instead."' do
server_def = ec2_server_create.config
expect(server_def[:tag_node_in_chef]).to eq(true)

Choose a reason for hiding this comment

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

@dheerajd-msys This test cases is not checking the deprecation message.

Copy link
Contributor

@btm btm left a comment

Choose a reason for hiding this comment

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

👍, but agree with @Vasu1105 that there's a missing expect in the test that ensures we're raising a deprecation message.

Signed-off-by: dheerajd-msys <dheeraj.dubey@msystechnologies.com>
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.

None yet

4 participants