From 8ad67050a3f2d4e421112bb7938cb86f3b3b5b0b Mon Sep 17 00:00:00 2001 From: mburns-r7 Date: Thu, 16 Nov 2017 17:45:24 +0000 Subject: [PATCH 1/5] fix #267 - add --retain for diff --- bin/convection | 4 +++- lib/convection/control/cloud.rb | 2 +- lib/convection/control/stack.rb | 4 ++-- lib/convection/model/template.rb | 13 +++++++++---- lib/convection/model/template/resource.rb | 5 ++++- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/bin/convection b/bin/convection index b7736ae..4714486 100755 --- a/bin/convection +++ b/bin/convection @@ -62,6 +62,7 @@ module Convection option :'very-verbose', :type => :boolean, :aliases => '--vv', :desc => 'Show unchanged stacks' option :cloudfiles, :type => :array, :default => %w(Cloudfile) option :delayed_output, :type => :boolean, :desc => 'Delay output until operation completion.', :default => false + option :retain, :type => :boolean, :desc => 'Retain stack resources, without deleteing.', :default => false def diff(stack = nil) @outputs = [] operation('diff', stack) @@ -101,6 +102,7 @@ module Convection @cloud.stacks[stack].validate end + desc 'describe-resources', 'Describe resources for a stack' option :cloudfile, :type => :string, :default => 'Cloudfile' option :stack, :desc => 'The stack to be described', :required => true @@ -133,7 +135,7 @@ module Convection cloud_array[:cloud].configure(File.absolute_path(cloud_array[:cloudfile_path], @cwd)) cloud = cloud_array[:cloud] region = cloud.cloudfile.region - cloud.send(task_name, stack, stack_group: options[:stack_group], stacks: options[:stacks], exclude_stacks: options[:exclude_stacks]) do |event, errors| + cloud.send(task_name, stack, stack_group: options[:stack_group], stacks: options[:stacks], exclude_stacks: options[:exclude_stacks], retain: options[:retain]) do |event, errors| if options[:cloudfiles].length > 1 && options[:delayed_output] output << { event: event, errors: errors } else diff --git a/lib/convection/control/cloud.rb b/lib/convection/control/cloud.rb index 8349b36..a9e7d4b 100644 --- a/lib/convection/control/cloud.rb +++ b/lib/convection/control/cloud.rb @@ -142,7 +142,7 @@ def diff(to_stack, options = {}, &block) filter_deck(options, &block).each_value do |stack| block.call(Model::Event.new(:compare, "Compare local state of stack #{ stack.name } (#{ stack.cloud_name }) with remote template", :info)) - difference = stack.diff + difference = stack.diff(retain: options[:retain]) # Find errors during diff emit_credential_error_and_exit!(stack, &block) if stack.credential_error? if stack.error? diff --git a/lib/convection/control/stack.rb b/lib/convection/control/stack.rb index 66b77ae..59cde8e 100644 --- a/lib/convection/control/stack.rb +++ b/lib/convection/control/stack.rb @@ -281,8 +281,8 @@ def to_json(pretty = false) # template (in CloudFormation) and the state of the rendered # template (what *would* be converged). # @see Convection::Model::Template#diff - def diff - @template.diff(@current_template) + def diff(retain: false) + @template.diff(@current_template, retain: retain) end # @return [Boolean] whether the Resources section of the rendered diff --git a/lib/convection/model/template.rb b/lib/convection/model/template.rb index b5cf50c..91da381 100644 --- a/lib/convection/model/template.rb +++ b/lib/convection/model/template.rb @@ -251,7 +251,7 @@ def execute end end - def render(stack_ = nil) + def render(stack_ = nil, retain: false) ## Instantiate a new template with the definition block and an other stack return clone(stack_).render unless stack_.nil? @@ -263,7 +263,12 @@ def render(stack_ = nil) 'Parameters' => parameters.map(&:render), 'Mappings' => mappings.map(&:render), 'Conditions' => conditions.map(&:render), - 'Resources' => all_resources.map(&:render), + 'Resources' => all_resources.map { |resource| + if retain && resource.deletion_policy == nil + resource.deletion_policy("Retain") + end + resource.render + }, 'Outputs' => outputs.map(&:render), 'Metadata' => metadata.map(&:render) } @@ -275,8 +280,8 @@ def all_resources end end - def diff(other, stack_ = nil) - render(stack_).diff(other).map { |diff| Diff.new(diff[0], *diff[1]) } + def diff(other, stack_ = nil, retain: false) + render(stack_, retain: retain).diff(other).map { |diff| Diff.new(diff[0], *diff[1]) } end def to_json(stack_ = nil, pretty = false) diff --git a/lib/convection/model/template/resource.rb b/lib/convection/model/template/resource.rb index 39e2985..b8b1ae4 100644 --- a/lib/convection/model/template/resource.rb +++ b/lib/convection/model/template/resource.rb @@ -326,9 +326,12 @@ def depends_on(resource) # rubocop:disable Style/TrivialAccessors # We don't want to use an accessor (e.g. deletion_policy=) because # this is a DSL method - def deletion_policy(deletion_policy) + def deletion_policy(deletion_policy = :unset_deletion_policy) + return @deletion_policy if deletion_policy == :unset_deletion_policy + @deletion_policy = deletion_policy end + # rubocop:enable Style/TrivialAccessors def reference From d1b30ea0c4daace66ef1caabb45e6b3bceab0634 Mon Sep 17 00:00:00 2001 From: mburns-r7 Date: Thu, 16 Nov 2017 17:45:24 +0000 Subject: [PATCH 2/5] fix #267 - add --retain for diff --- bin/convection | 4 +++- lib/convection/control/cloud.rb | 2 +- lib/convection/control/stack.rb | 4 ++-- lib/convection/model/template.rb | 13 +++++++++---- lib/convection/model/template/resource.rb | 5 ++++- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/bin/convection b/bin/convection index b7736ae..4714486 100755 --- a/bin/convection +++ b/bin/convection @@ -62,6 +62,7 @@ module Convection option :'very-verbose', :type => :boolean, :aliases => '--vv', :desc => 'Show unchanged stacks' option :cloudfiles, :type => :array, :default => %w(Cloudfile) option :delayed_output, :type => :boolean, :desc => 'Delay output until operation completion.', :default => false + option :retain, :type => :boolean, :desc => 'Retain stack resources, without deleteing.', :default => false def diff(stack = nil) @outputs = [] operation('diff', stack) @@ -101,6 +102,7 @@ module Convection @cloud.stacks[stack].validate end + desc 'describe-resources', 'Describe resources for a stack' option :cloudfile, :type => :string, :default => 'Cloudfile' option :stack, :desc => 'The stack to be described', :required => true @@ -133,7 +135,7 @@ module Convection cloud_array[:cloud].configure(File.absolute_path(cloud_array[:cloudfile_path], @cwd)) cloud = cloud_array[:cloud] region = cloud.cloudfile.region - cloud.send(task_name, stack, stack_group: options[:stack_group], stacks: options[:stacks], exclude_stacks: options[:exclude_stacks]) do |event, errors| + cloud.send(task_name, stack, stack_group: options[:stack_group], stacks: options[:stacks], exclude_stacks: options[:exclude_stacks], retain: options[:retain]) do |event, errors| if options[:cloudfiles].length > 1 && options[:delayed_output] output << { event: event, errors: errors } else diff --git a/lib/convection/control/cloud.rb b/lib/convection/control/cloud.rb index 8349b36..a9e7d4b 100644 --- a/lib/convection/control/cloud.rb +++ b/lib/convection/control/cloud.rb @@ -142,7 +142,7 @@ def diff(to_stack, options = {}, &block) filter_deck(options, &block).each_value do |stack| block.call(Model::Event.new(:compare, "Compare local state of stack #{ stack.name } (#{ stack.cloud_name }) with remote template", :info)) - difference = stack.diff + difference = stack.diff(retain: options[:retain]) # Find errors during diff emit_credential_error_and_exit!(stack, &block) if stack.credential_error? if stack.error? diff --git a/lib/convection/control/stack.rb b/lib/convection/control/stack.rb index 477cec4..2d7d164 100644 --- a/lib/convection/control/stack.rb +++ b/lib/convection/control/stack.rb @@ -279,8 +279,8 @@ def to_json(pretty = false) # template (in CloudFormation) and the state of the rendered # template (what *would* be converged). # @see Convection::Model::Template#diff - def diff - @template.diff(@current_template) + def diff(retain: false) + @template.diff(@current_template, retain: retain) end # @return [Boolean] whether the Resources section of the rendered diff --git a/lib/convection/model/template.rb b/lib/convection/model/template.rb index b5cf50c..91da381 100644 --- a/lib/convection/model/template.rb +++ b/lib/convection/model/template.rb @@ -251,7 +251,7 @@ def execute end end - def render(stack_ = nil) + def render(stack_ = nil, retain: false) ## Instantiate a new template with the definition block and an other stack return clone(stack_).render unless stack_.nil? @@ -263,7 +263,12 @@ def render(stack_ = nil) 'Parameters' => parameters.map(&:render), 'Mappings' => mappings.map(&:render), 'Conditions' => conditions.map(&:render), - 'Resources' => all_resources.map(&:render), + 'Resources' => all_resources.map { |resource| + if retain && resource.deletion_policy == nil + resource.deletion_policy("Retain") + end + resource.render + }, 'Outputs' => outputs.map(&:render), 'Metadata' => metadata.map(&:render) } @@ -275,8 +280,8 @@ def all_resources end end - def diff(other, stack_ = nil) - render(stack_).diff(other).map { |diff| Diff.new(diff[0], *diff[1]) } + def diff(other, stack_ = nil, retain: false) + render(stack_, retain: retain).diff(other).map { |diff| Diff.new(diff[0], *diff[1]) } end def to_json(stack_ = nil, pretty = false) diff --git a/lib/convection/model/template/resource.rb b/lib/convection/model/template/resource.rb index 39e2985..b8b1ae4 100644 --- a/lib/convection/model/template/resource.rb +++ b/lib/convection/model/template/resource.rb @@ -326,9 +326,12 @@ def depends_on(resource) # rubocop:disable Style/TrivialAccessors # We don't want to use an accessor (e.g. deletion_policy=) because # this is a DSL method - def deletion_policy(deletion_policy) + def deletion_policy(deletion_policy = :unset_deletion_policy) + return @deletion_policy if deletion_policy == :unset_deletion_policy + @deletion_policy = deletion_policy end + # rubocop:enable Style/TrivialAccessors def reference From 2f6fb990389ea033c3840db350b749cb2af89763 Mon Sep 17 00:00:00 2001 From: mburns-r7 Date: Mon, 20 Nov 2017 16:33:50 +0000 Subject: [PATCH 3/5] added retain flag for converge --- bin/convection | 10 ++++++++-- lib/convection/control/cloud.rb | 2 +- lib/convection/control/stack.rb | 9 ++++----- lib/convection/model/template.rb | 12 ++++++------ lib/convection/model/template/resource.rb | 3 --- 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/bin/convection b/bin/convection index 4714486..8f3ca6a 100755 --- a/bin/convection +++ b/bin/convection @@ -23,6 +23,7 @@ module Convection option :'very-verbose', :type => :boolean, :aliases => '--vv', :desc => 'Show unchanged stacks', default: true option :cloudfiles, :type => :array, :default => %w(Cloudfile) option :delayed_output, :type => :boolean, :desc => 'Delay output until operation completion.', :default => false + option :retain, :type => :boolean, :desc => 'Retain stack resources, without deleteing.', :default => false def converge(stack = nil) @outputs = [] operation('converge', stack) @@ -102,7 +103,6 @@ module Convection @cloud.stacks[stack].validate end - desc 'describe-resources', 'Describe resources for a stack' option :cloudfile, :type => :string, :default => 'Cloudfile' option :stack, :desc => 'The stack to be described', :required => true @@ -135,7 +135,13 @@ module Convection cloud_array[:cloud].configure(File.absolute_path(cloud_array[:cloudfile_path], @cwd)) cloud = cloud_array[:cloud] region = cloud.cloudfile.region - cloud.send(task_name, stack, stack_group: options[:stack_group], stacks: options[:stacks], exclude_stacks: options[:exclude_stacks], retain: options[:retain]) do |event, errors| + operation_kwargs = { + stack_group: options[:stack_group], + stacks: options[:stacks], + exclude_stacks: options[:exclude_stacks], + retain: options[:retain] + } + cloud.send(task_name, stack, operation_kwargs) do |event, errors| if options[:cloudfiles].length > 1 && options[:delayed_output] output << { event: event, errors: errors } else diff --git a/lib/convection/control/cloud.rb b/lib/convection/control/cloud.rb index a9e7d4b..a38c929 100644 --- a/lib/convection/control/cloud.rb +++ b/lib/convection/control/cloud.rb @@ -91,7 +91,7 @@ def converge(to_stack, options = {}, &block) filter_deck(options, &block).each_value do |stack| block.call(Model::Event.new(:converge, "Stack #{ stack.name }", :info)) if block - stack.apply(&block) + stack.apply(retain: options[:retain], &block) emit_credential_error_and_exit!(stack, &block) if stack.credential_error? if stack.error? diff --git a/lib/convection/control/stack.rb b/lib/convection/control/stack.rb index 2d7d164..538b2e5 100644 --- a/lib/convection/control/stack.rb +++ b/lib/convection/control/stack.rb @@ -147,7 +147,6 @@ def initialize(name, template, options = {}, &block) # clouds use this, for example, to create security groups early # in the dependency tree to avoid the chicken-and-egg problem. @template.execute - rescue Aws::Errors::ServiceError => e @errors << e end @@ -322,9 +321,9 @@ def resource_dependent_changes? # # @param block [Proc] a configuration block to pass any # {Convection::Model::Event}s to. - def apply(&block) + def apply(retain: false, &block) request_options = @options.clone.tap do |o| - o[:template_body] = to_json + o[:template_body] = to_json(retain: retain) o[:parameters] = cf_parameters o[:capabilities] = capabilities end @@ -332,7 +331,7 @@ def apply(&block) # Get the state of existence before creation existing_stack = exist? if existing_stack - if diff.empty? ## No Changes. Just get resources and move on + if diff(retain: retain).empty? ## No Changes. Just get resources and move on block.call(Model::Event.new(:complete, "Stack #{ name } has no changes", :info)) if block get_status return @@ -552,7 +551,7 @@ def get_events(pages = nil, stack_name = id) collection << event end - break if pages == 0 + break if pages.zero? end @last_event_seen = collection.first.event_id unless collection.empty? diff --git a/lib/convection/model/template.rb b/lib/convection/model/template.rb index 91da381..1de2411 100644 --- a/lib/convection/model/template.rb +++ b/lib/convection/model/template.rb @@ -263,12 +263,12 @@ def render(stack_ = nil, retain: false) 'Parameters' => parameters.map(&:render), 'Mappings' => mappings.map(&:render), 'Conditions' => conditions.map(&:render), - 'Resources' => all_resources.map { |resource| - if retain && resource.deletion_policy == nil - resource.deletion_policy("Retain") + 'Resources' => all_resources.map do |resource| + if retain && resource.deletion_policy.nil? + resource.deletion_policy('Retain') end resource.render - }, + end, 'Outputs' => outputs.map(&:render), 'Metadata' => metadata.map(&:render) } @@ -284,8 +284,8 @@ def diff(other, stack_ = nil, retain: false) render(stack_, retain: retain).diff(other).map { |diff| Diff.new(diff[0], *diff[1]) } end - def to_json(stack_ = nil, pretty = false) - rendered_stack = render(stack_) + def to_json(stack_ = nil, pretty = false, retain: false) + rendered_stack = render(stack_, retain: retain) validate(rendered_stack) return JSON.generate(rendered_stack) unless pretty JSON.pretty_generate(rendered_stack) diff --git a/lib/convection/model/template/resource.rb b/lib/convection/model/template/resource.rb index b8b1ae4..e1d5646 100644 --- a/lib/convection/model/template/resource.rb +++ b/lib/convection/model/template/resource.rb @@ -323,7 +323,6 @@ def depends_on(resource) @depends_on << (resource.is_a?(Resource) ? resource.name : resource) end - # rubocop:disable Style/TrivialAccessors # We don't want to use an accessor (e.g. deletion_policy=) because # this is a DSL method def deletion_policy(deletion_policy = :unset_deletion_policy) @@ -332,8 +331,6 @@ def deletion_policy(deletion_policy = :unset_deletion_policy) @deletion_policy = deletion_policy end - # rubocop:enable Style/TrivialAccessors - def reference { 'Ref' => name From 954293d169dcfac96930248b8fb37447c7ebbe53 Mon Sep 17 00:00:00 2001 From: mburns-r7 Date: Mon, 20 Nov 2017 17:05:00 +0000 Subject: [PATCH 4/5] Changed formatting to comply with rubocop --- bin/convection | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bin/convection b/bin/convection index 8f3ca6a..315f9fe 100755 --- a/bin/convection +++ b/bin/convection @@ -136,10 +136,8 @@ module Convection cloud = cloud_array[:cloud] region = cloud.cloudfile.region operation_kwargs = { - stack_group: options[:stack_group], - stacks: options[:stacks], - exclude_stacks: options[:exclude_stacks], - retain: options[:retain] + stack_group: options[:stack_group], stacks: options[:stacks], + exclude_stacks: options[:exclude_stacks], retain: options[:retain] } cloud.send(task_name, stack, operation_kwargs) do |event, errors| if options[:cloudfiles].length > 1 && options[:delayed_output] From 0a185f2114f3ee494844f9a3c75b88e5de77d6f1 Mon Sep 17 00:00:00 2001 From: mburns-r7 Date: Mon, 20 Nov 2017 17:08:04 +0000 Subject: [PATCH 5/5] rubocop rule update --- .rubocop_todo.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bdc0222..8ec22f2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -59,7 +59,7 @@ Metrics/AbcSize: # Offense count: 34 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 136 + Max: 140 # Offense count: 5 # Configuration parameters: CountComments.