From 8d049171cc5339193ee6c9c1f2c519367d5c6364 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 17:39:51 +0000 Subject: [PATCH 01/22] moving terraform custom_update_properties to core --- provider/core.rb | 11 +++++++++++ provider/terraform.rb | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/provider/core.rb b/provider/core.rb index bb5c268a293e..37567e229814 100644 --- a/provider/core.rb +++ b/provider/core.rb @@ -753,5 +753,16 @@ def effective_copyright_year(out_file) end Time.now.year end + + # Filter the properties to keep only the ones requiring custom update + # method and group them by update url & verb. + def properties_by_custom_update(properties) + update_props = properties.reject do |p| + p.update_url.nil? || p.update_verb.nil? + end + update_props.group_by do |p| + { update_url: p.update_url, update_verb: p.update_verb } + end + end end end diff --git a/provider/terraform.rb b/provider/terraform.rb index f15382ae7afa..0f3055fd127a 100644 --- a/provider/terraform.rb +++ b/provider/terraform.rb @@ -108,17 +108,6 @@ def nested_properties(property) end end - # Filter the properties to keep only the ones requiring custom update - # method and group them by update url & verb. - def properties_by_custom_update(properties) - update_props = properties.reject do |p| - p.update_url.nil? || p.update_verb.nil? - end - update_props.group_by do |p| - { update_url: p.update_url, update_verb: p.update_verb } - end - end - private # This function uses the resource.erb template to create one file From 00c5aa55d753fe9ebf5927623142ec13e516ee0f Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 18:25:10 +0000 Subject: [PATCH 02/22] Per field update function in Ansible templates --- templates/ansible/resource.erb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index 2c1e368446f5..56d4e4c40439 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -16,6 +16,8 @@ __metaclass__ = type metadata_version = quote_string(@config.manifest.get('metadata_version', object)) supported_by = quote_string(@config.manifest.get('supported_by', object)) + supported_by = quote_string(@config.manifest.get('supported_by', config)) + update_props = properties_by_custom_update(object.all_user_properties) -%> ANSIBLE_METADATA = {'metadata_version': <%= metadata_version -%>, 'status': <%= @config.manifest.get('status', object) -%>, @@ -263,6 +265,25 @@ def main(): <% end # object.update.nil? -%> +<% unless update_props.empty? -%> +def update_fields(request, response): + difference = GcpDifference(request).difference(GcpDifference(response)) + auth = GcpSession(module, <%= quote_string(prod_name) -%>) +<% update_props.each do |key, props| -%> + if <%= props.map { |prop| "difference.get(#{quote_string(Google::StringUtils.underscore(prop.name))})" }.join(' || ') -%>: + auth.<%= key[:update_verb].downcase -%>( + ''.join([ + "<%= object.__product.base_url -%>", + "<%= key[:update_url].gsub('{{', '{').gsub('}}', '}') -%>" + ]).format(**module.params), + { +<%= lines(indent_list(props.map { |p| "#{Google::StringUtils.underscore(p.name)}: module.params('#{Google::StringUtils.underscore(p.name)}')" }, 12)) -%> + } + ) +<% end # update_props.each -%> + + +<% end # unless update_props.empty? -%> <%= lines(method_decl('delete', ['module', 'link', ('kind' if object.kind?), ('fetch' if object.access_api_results)])) From 58e1bf6e3074baf2f084a38e678ef83881127abf Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 19:04:38 +0000 Subject: [PATCH 03/22] Adding method calls --- templates/ansible/resource.erb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index 56d4e4c40439..894cfeffee73 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -155,6 +155,9 @@ def main(): ('fetch' if object.access_api_results) ]) -%> +<% unless update_props.empty? -%> + update_fields(resource_to_request(module), fetch_to_hash(fetch)) +<% end -%> <%= lines(indent("fetch = #{method}", 16)) -%> changed = True else: @@ -242,7 +245,7 @@ def main(): end -%> <% if object.update.nil? -%> -<% if !false?(object.editable) -%> +<% if !false?(object.editable) && update_props.empty? -%> auth = GcpSession(module, <%= quote_string(prod_name) -%>) <% update_verb = object.update_verb.to_s.downcase From f79c3421fe2a93e5893086fd9e002c6fafdc8717 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 19:25:48 +0000 Subject: [PATCH 04/22] Getting sanity tests to pass --- templates/ansible/resource.erb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index 894cfeffee73..c8ef113f9770 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -156,7 +156,8 @@ def main(): ]) -%> <% unless update_props.empty? -%> - update_fields(resource_to_request(module), fetch_to_hash(fetch)) + update_fields(module, resource_to_request(module), + response_to_hash(module, fetch)) <% end -%> <%= lines(indent("fetch = #{method}", 16)) -%> changed = True @@ -269,19 +270,19 @@ def main(): <% unless update_props.empty? -%> -def update_fields(request, response): - difference = GcpDifference(request).difference(GcpDifference(response)) +def update_fields(module, request, response): + difference = GcpRequest(request).difference(GcpRequest(response)) auth = GcpSession(module, <%= quote_string(prod_name) -%>) <% update_props.each do |key, props| -%> if <%= props.map { |prop| "difference.get(#{quote_string(Google::StringUtils.underscore(prop.name))})" }.join(' || ') -%>: auth.<%= key[:update_verb].downcase -%>( - ''.join([ - "<%= object.__product.base_url -%>", - "<%= key[:update_url].gsub('{{', '{').gsub('}}', '}') -%>" - ]).format(**module.params), - { -<%= lines(indent_list(props.map { |p| "#{Google::StringUtils.underscore(p.name)}: module.params('#{Google::StringUtils.underscore(p.name)}')" }, 12)) -%> - } + ''.join([ + "<%= object.__product.base_url -%>", + "<%= key[:update_url].gsub('{{', '{').gsub('}}', '}') -%>" + ]).format(**module.params), + { +<%= lines(indent_list(props.map { |p| "#{quote_string(Google::StringUtils.underscore(p.name))}: module.params('#{Google::StringUtils.underscore(p.name)}')" }, 16)) -%> + } ) <% end # update_props.each -%> From 7bb489ee383d9657496f8c88dccdcddf903bbb63 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 19:27:33 +0000 Subject: [PATCH 05/22] Resource is updatable if it has updatable properties --- templates/ansible/resource.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index c8ef113f9770..a8e4c51524f5 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -246,7 +246,7 @@ def main(): end -%> <% if object.update.nil? -%> -<% if !false?(object.editable) && update_props.empty? -%> +<% if object.editable || !update_props.empty? -%> auth = GcpSession(module, <%= quote_string(prod_name) -%>) <% update_verb = object.update_verb.to_s.downcase From 42bcff2374bc0e3420eefadb12fcc0f68f9fc3a9 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 19:36:25 +0000 Subject: [PATCH 06/22] use the resource_to_request code I have already to get resourceref + array support --- templates/ansible/resource.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index a8e4c51524f5..caf4984d7a0b 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -281,7 +281,7 @@ def update_fields(module, request, response): "<%= key[:update_url].gsub('{{', '{').gsub('}}', '}') -%>" ]).format(**module.params), { -<%= lines(indent_list(props.map { |p| "#{quote_string(Google::StringUtils.underscore(p.name))}: module.params('#{Google::StringUtils.underscore(p.name)}')" }, 16)) -%> +<%= lines(indent(request_properties(props), 12)) -%> } ) <% end # update_props.each -%> From e665bfa9bc60af17c314e73d683d4db278c38368 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 20:32:21 +0000 Subject: [PATCH 07/22] moving properties_by_custom_update to non-private --- provider/core.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/provider/core.rb b/provider/core.rb index 37567e229814..ec87e0ed9bf7 100644 --- a/provider/core.rb +++ b/provider/core.rb @@ -568,6 +568,17 @@ def relative_path(target, base) Pathname.new(target).relative_path_from(Pathname.new(base)) end + # Filter the properties to keep only the ones requiring custom update + # method and group them by update url & verb. + def properties_by_custom_update(properties) + update_props = properties.reject do |p| + p.update_url.nil? || p.update_verb.nil? + end + update_props.group_by do |p| + { update_url: p.update_url, update_verb: p.update_verb } + end + end + # TODO(nelsonjr): Review all object interfaces and move to private methods # that should not be exposed outside the object hierarchy. private @@ -753,16 +764,5 @@ def effective_copyright_year(out_file) end Time.now.year end - - # Filter the properties to keep only the ones requiring custom update - # method and group them by update url & verb. - def properties_by_custom_update(properties) - update_props = properties.reject do |p| - p.update_url.nil? || p.update_verb.nil? - end - update_props.group_by do |p| - { update_url: p.update_url, update_verb: p.update_verb } - end - end end end From 40f9c217b92b46576bcbf40534d3320d17cc61ea Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 21:45:49 +0000 Subject: [PATCH 08/22] Puppet partial updates --- provider/core.rb | 5 +++++ provider/terraform.rb | 5 ----- templates/puppet/resource.erb | 13 +++++++++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/provider/core.rb b/provider/core.rb index ec87e0ed9bf7..c694065191b9 100644 --- a/provider/core.rb +++ b/provider/core.rb @@ -579,6 +579,11 @@ def properties_by_custom_update(properties) end end + def update_url(resource, url_part) + return self_link_url(resource) if url_part.nil? + [resource.__product.base_url, url_part].flatten.join + end + # TODO(nelsonjr): Review all object interfaces and move to private methods # that should not be exposed outside the object hierarchy. private diff --git a/provider/terraform.rb b/provider/terraform.rb index 0f3055fd127a..d58f7e334537 100644 --- a/provider/terraform.rb +++ b/provider/terraform.rb @@ -71,11 +71,6 @@ def build_url(url_parts, _extra = false) url_parts.flatten.join end - def update_url(resource, url_part) - return build_url(resource.self_link_url) if url_part.nil? - [resource.__product.base_url, url_part].flatten.join - end - # Transforms a format string with field markers to a regex string with # capture groups. # diff --git a/templates/puppet/resource.erb b/templates/puppet/resource.erb index 795925d7830b..24d52d1dca45 100644 --- a/templates/puppet/resource.erb +++ b/templates/puppet/resource.erb @@ -317,6 +317,19 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do <% # TODO(nelsonjr): Remove @dirty or SQL does not do idempotent updates. -%> # return on !@dirty is for aiding testing (puppet already guarantees that) return if @created || @deleted || !@dirty +<% update_props = properties_by_custom_update(object.all_user_properties) -%> +<% update_props.each do |key, props| -%> + if <%= props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') -%>: + Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( + <%= update_url(object, key[:update_url]) -%>, + { +<%= lines(indent_list(props.map { |prop| "#{prop.api_name}: @resource[:#{prop.out_name}]" }, 10)) -%> + }, + fetch_auth(@resource), + 'application/json' + ) + end +<% end # update_props.each -%> <% if object&.handlers&.flush.nil? -%> <% put_new = ["Google::#{product_ns}::Network", From 36d0a296c50c6c1338af08d500b024ff1f8214ad Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 21:52:08 +0000 Subject: [PATCH 09/22] Partial update on Chef --- templates/chef/resource.erb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/templates/chef/resource.erb b/templates/chef/resource.erb index 978ed0a340e1..663fdea3486a 100644 --- a/templates/chef/resource.erb +++ b/templates/chef/resource.erb @@ -436,6 +436,19 @@ module Google puts # making a newline until we find a better way TODO: find! compute_changes.each { |log| puts " - #{log.strip}\n" } <% if object&.handlers&.update.nil? -%> +<% update_props = properties_by_custom_update(object.all_user_properties) -%> +<% update_props.each do |key, props| -%> + if <%= props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') -%>: + Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( + <%= update_url(object, key[:update_url]) -%>, + { +<%= lines(indent_list(props.map { |prop| "#{prop.api_name}: @resource[:#{prop.out_name}]" }, 10)) -%> + }, + fetch_auth(@resource), + 'application/json' + ) + end +<% end # update_props.each -%> <% put_new = ["::Google::#{product_ns}::Network", "#{object.update_verb.to_s.capitalize}.new" From 634a111d9ca446e94e50eaf54f67f1dab0ec9a41 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 27 Jul 2018 21:57:37 +0000 Subject: [PATCH 10/22] Chef is missing some encoders --- products/compute/chef.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/products/compute/chef.yaml b/products/compute/chef.yaml index 78909b14c9f3..8ace7dc01245 100644 --- a/products/compute/chef.yaml +++ b/products/compute/chef.yaml @@ -49,6 +49,9 @@ overrides: !ruby/object:Provider::ResourceOverrides message = 'Instance cannot be edited' Chef::Log.fatal message raise message + provider_helpers: + - 'products/compute/helpers/ruby/provider_instance.rb' + - 'products/compute/helpers/ruby/instance_metadata.rb' Network: !ruby/object:Provider::Chef::ResourceOverride handlers: !ruby/object:Provider::Chef::Handlers # TODO(alexstephen): Better editing from auto to custom From 8061fc980e49566c79e5ca82e3ac7562cc4034d5 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 21:46:52 +0000 Subject: [PATCH 11/22] Fingerprint and fixing the broken partial updates --- api/type.rb | 20 ++++++++++---------- products/compute/api.yaml | 3 --- templates/puppet/resource.erb | 15 ++++++++++++--- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/api/type.rb b/api/type.rb index 5f55c25e5468..f093d9a910ee 100644 --- a/api/type.rb +++ b/api/type.rb @@ -166,9 +166,18 @@ def initialize(name) NAME = Api::Type::String.new('name') end + # Properties that are fetched externally + class FetchedExternal < Type + attr_writer :resource + + def api_name + name + end + end + # Represents a fingerprint. A fingerprint is an output-only # field used for optimistic locking during updates. - class Fingerprint < String + class Fingerprint < FetchedExternal def validate super @output = true if @output.nil? @@ -304,15 +313,6 @@ def validate end end - # Properties that are fetched externally - class FetchedExternal < Type - attr_writer :resource - - def api_name - name - end - end - # Represents a 'selfLink' property, which returns the URI of the resource. class SelfLink < FetchedExternal EXPORT_KEY = 'selfLink'.freeze diff --git a/products/compute/api.yaml b/products/compute/api.yaml index 4dd4f7d42a8a..3a33097fa3fd 100644 --- a/products/compute/api.yaml +++ b/products/compute/api.yaml @@ -146,7 +146,6 @@ objects: internally during updates. update_url: 'projects/{{project}}/regions/{{region}}/addresses/{{name}}/setLabels' update_verb: :POST - min_version: beta - !ruby/object:Api::Resource name: 'Autoscaler' kind: 'compute#autoscaler' @@ -900,7 +899,6 @@ objects: internally during updates. update_url: 'projects/{{project}}/regions/{{region}}/forwardingRules/{{name}}/setLabels' update_verb: :POST - min_version: beta - !ruby/object:Api::Type::Enum name: 'networkTier' description: | @@ -1000,7 +998,6 @@ objects: internally during updates. update_url: 'projects/{{project}}/global/addresses/{{name}}/setLabels' update_verb: :POST - min_version: beta - !ruby/object:Api::Type::Enum name: 'ipVersion' description: | diff --git a/templates/puppet/resource.erb b/templates/puppet/resource.erb index 24d52d1dca45..b7a14de61a66 100644 --- a/templates/puppet/resource.erb +++ b/templates/puppet/resource.erb @@ -319,11 +319,20 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do return if @created || @deleted || !@dirty <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> - if <%= props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') -%>: + if <%= props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') %> Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( - <%= update_url(object, key[:update_url]) -%>, + <%= quote_string(update_url(object, key[:update_url])) -%>, { -<%= lines(indent_list(props.map { |prop| "#{prop.api_name}: @resource[:#{prop.out_name}]" }, 10)) -%> +<% + update_prop_statements = props.map do |prop| + if prop.is_a? Api::Type::FetchedExternal + "#{prop.api_name}: @fetched['#{prop.api_name}']" + else + "#{prop.api_name}: @resource[:#{prop.out_name}]" + end + end +-%> +<%= lines(indent_list(update_prop_statements, 10)) -%> }, fetch_auth(@resource), 'application/json' From 750f2ce8458e1435adfba18a0a3246b4abb48908 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:02:49 +0000 Subject: [PATCH 12/22] reorganizing puppet updates for better sanity and to make them actually work --- templates/puppet/resource.erb | 22 +++------------------- templates/puppetchef/update_props.erb | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 19 deletions(-) create mode 100644 templates/puppetchef/update_props.erb diff --git a/templates/puppet/resource.erb b/templates/puppet/resource.erb index b7a14de61a66..7e8debe813e2 100644 --- a/templates/puppet/resource.erb +++ b/templates/puppet/resource.erb @@ -319,25 +319,8 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do return if @created || @deleted || !@dirty <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> - if <%= props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') %> - Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( - <%= quote_string(update_url(object, key[:update_url])) -%>, - { -<% - update_prop_statements = props.map do |prop| - if prop.is_a? Api::Type::FetchedExternal - "#{prop.api_name}: @fetched['#{prop.api_name}']" - else - "#{prop.api_name}: @resource[:#{prop.out_name}]" - end - end --%> -<%= lines(indent_list(update_prop_statements, 10)) -%> - }, - fetch_auth(@resource), - 'application/json' - ) - end +<% prop_statement = props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') -%> + <%= props.first.name.downcase -%>_update(@resource) if <%= prop_statement %> <% end # update_props.each -%> <% if object&.handlers&.flush.nil? -%> <% @@ -374,6 +357,7 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do } end +<%= lines(compile('templates/puppetchef/update_props.erb')) -%> <% unless object.exports.nil? -%> <% exp_list = [ diff --git a/templates/puppetchef/update_props.erb b/templates/puppetchef/update_props.erb new file mode 100644 index 000000000000..ebfd93d59ce8 --- /dev/null +++ b/templates/puppetchef/update_props.erb @@ -0,0 +1,23 @@ +<% update_props = properties_by_custom_update(object.all_user_properties) -%> +<% update_props.each do |key, props| -%> +def <%= props[0].name.downcase -%>_update(data) + Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( +<%= indent(build_url(object.__product.base_url, key[:update_url]), 4) -%>, + { +<% + update_prop_statements = props.map do |prop| + if prop.is_a? Api::Type::FetchedExternal + "#{prop.api_name}: @fetched['#{prop.api_name}']" + else + "#{prop.api_name}: @resource[:#{prop.out_name}]" + end + end +-%> +<%= lines(indent_list(update_prop_statements, 6)) -%> + }, + fetch_auth(@resource), + 'application/json' + ) +end + +<% end # update_props.each -%> From 76ff25c5d2793d7bfa9dfc60e0b07c5495aba7e5 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:17:41 +0000 Subject: [PATCH 13/22] partial updates work --- templates/puppet/resource.erb | 4 ++-- templates/puppetchef/update_props.erb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/puppet/resource.erb b/templates/puppet/resource.erb index 7e8debe813e2..e58ab5b6033d 100644 --- a/templates/puppet/resource.erb +++ b/templates/puppet/resource.erb @@ -319,7 +319,7 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do return if @created || @deleted || !@dirty <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> -<% prop_statement = props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') -%> +<% prop_statement = props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)}]" }.join(' || ') -%> <%= props.first.name.downcase -%>_update(@resource) if <%= prop_statement %> <% end # update_props.each -%> <% if object&.handlers&.flush.nil? -%> @@ -357,7 +357,7 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do } end -<%= lines(compile('templates/puppetchef/update_props.erb')) -%> +<%= lines(indent(compile('templates/puppetchef/update_props.erb'), 2)) -%> <% unless object.exports.nil? -%> <% exp_list = [ diff --git a/templates/puppetchef/update_props.erb b/templates/puppetchef/update_props.erb index ebfd93d59ce8..e3ed4bd8bcc3 100644 --- a/templates/puppetchef/update_props.erb +++ b/templates/puppetchef/update_props.erb @@ -3,6 +3,8 @@ def <%= props[0].name.downcase -%>_update(data) Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( <%= indent(build_url(object.__product.base_url, key[:update_url]), 4) -%>, + fetch_auth(@resource), + 'application/json', { <% update_prop_statements = props.map do |prop| @@ -14,10 +16,8 @@ def <%= props[0].name.downcase -%>_update(data) end -%> <%= lines(indent_list(update_prop_statements, 6)) -%> - }, - fetch_auth(@resource), - 'application/json' - ) + }.to_json + ).send end <% end # update_props.each -%> From 422fc0626dd38c12f674fec5cdf9d5faf12efb9d Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:25:48 +0000 Subject: [PATCH 14/22] chef --- templates/chef/resource.erb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/templates/chef/resource.erb b/templates/chef/resource.erb index 663fdea3486a..9527712dd122 100644 --- a/templates/chef/resource.erb +++ b/templates/chef/resource.erb @@ -438,18 +438,10 @@ module Google <% if object&.handlers&.update.nil? -%> <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> - if <%= props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)})]" }.join(' || ') -%>: - Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( - <%= update_url(object, key[:update_url]) -%>, - { -<%= lines(indent_list(props.map { |prop| "#{prop.api_name}: @resource[:#{prop.out_name}]" }, 10)) -%> - }, - fetch_auth(@resource), - 'application/json' - ) - end + <% prop_statement = props.map { |prop| "if (@current_resource.#{prop.name.underscore} != @new_resource.#{prop.name.underscore})" }.join(' || ') -%> + <%= props.first.name.downcase -%>_update(@resource) if <%= prop_statement %> <% end # update_props.each -%> -<% +<% put_new = ["::Google::#{product_ns}::Network", "#{object.update_verb.to_s.capitalize}.new" ].join('::') From c84f1957fd89062d90ed8541973f8b56cba22b9f Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:27:45 +0000 Subject: [PATCH 15/22] no more stringutils --- templates/ansible/resource.erb | 2 +- templates/puppet/resource.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index caf4984d7a0b..67e59f4ca7af 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -274,7 +274,7 @@ def update_fields(module, request, response): difference = GcpRequest(request).difference(GcpRequest(response)) auth = GcpSession(module, <%= quote_string(prod_name) -%>) <% update_props.each do |key, props| -%> - if <%= props.map { |prop| "difference.get(#{quote_string(Google::StringUtils.underscore(prop.name))})" }.join(' || ') -%>: + if <%= props.map { |prop| "difference.get(#{quote_string(prop.name.underscore))})" }.join(' || ') -%>: auth.<%= key[:update_verb].downcase -%>( ''.join([ "<%= object.__product.base_url -%>", diff --git a/templates/puppet/resource.erb b/templates/puppet/resource.erb index e58ab5b6033d..812e5db8b6e6 100644 --- a/templates/puppet/resource.erb +++ b/templates/puppet/resource.erb @@ -319,7 +319,7 @@ Puppet::Type.type(:<%= object.out_name -%>).provide(:google) do return if @created || @deleted || !@dirty <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> -<% prop_statement = props.map { |prop| "@dirty[:#{Google::StringUtils.underscore(prop.name)}]" }.join(' || ') -%> +<% prop_statement = props.map { |prop| "@dirty[:#{prop.name.underscore}]" }.join(' || ') -%> <%= props.first.name.downcase -%>_update(@resource) if <%= prop_statement %> <% end # update_props.each -%> <% if object&.handlers&.flush.nil? -%> From e608eb61ef12294a2efda89717a75f90cb517a7b Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:29:06 +0000 Subject: [PATCH 16/22] chef partial update --- templates/chef/resource.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/templates/chef/resource.erb b/templates/chef/resource.erb index 9527712dd122..3625c7d9299f 100644 --- a/templates/chef/resource.erb +++ b/templates/chef/resource.erb @@ -439,7 +439,9 @@ module Google <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> <% prop_statement = props.map { |prop| "if (@current_resource.#{prop.name.underscore} != @new_resource.#{prop.name.underscore})" }.join(' || ') -%> - <%= props.first.name.downcase -%>_update(@resource) if <%= prop_statement %> + if <%= prop_statement -%> + <%= props.first.name.downcase -%>_update(@current_resource) + end <% end # update_props.each -%> <% put_new = ["::Google::#{product_ns}::Network", From c4ec72570d0d56218f1a9b71104d8608bdad721d Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:46:46 +0000 Subject: [PATCH 17/22] Chef seems to work --- templates/chef/resource.erb | 5 +++-- templates/puppetchef/update_props.erb | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/templates/chef/resource.erb b/templates/chef/resource.erb index 3625c7d9299f..e06d758b5cb5 100644 --- a/templates/chef/resource.erb +++ b/templates/chef/resource.erb @@ -438,8 +438,8 @@ module Google <% if object&.handlers&.update.nil? -%> <% update_props = properties_by_custom_update(object.all_user_properties) -%> <% update_props.each do |key, props| -%> - <% prop_statement = props.map { |prop| "if (@current_resource.#{prop.name.underscore} != @new_resource.#{prop.name.underscore})" }.join(' || ') -%> - if <%= prop_statement -%> +<% prop_statement = props.map { |prop| "(@current_resource.#{prop.name.underscore} != @new_resource.#{prop.name.underscore})" }.join(' || ') -%> + if <%= prop_statement %> <%= props.first.name.downcase -%>_update(@current_resource) end <% end # update_props.each -%> @@ -512,6 +512,7 @@ module Google <%= lines(indent(emit_method('self.resource_to_hash', %w[resource], r2h_code, file_relative), 8)) -%> +<%= lines(indent(compile('templates/puppetchef/update_props.erb'), 2)) -%> # Copied from Chef > Provider > #converge_if_changed def compute_changes properties = @new_resource.class.state_properties.map(&:name) diff --git a/templates/puppetchef/update_props.erb b/templates/puppetchef/update_props.erb index e3ed4bd8bcc3..14a781627211 100644 --- a/templates/puppetchef/update_props.erb +++ b/templates/puppetchef/update_props.erb @@ -2,7 +2,7 @@ <% update_props.each do |key, props| -%> def <%= props[0].name.downcase -%>_update(data) Google::<%= product_ns -%>::Network::<%= key[:update_verb].capitalize -%>.new( -<%= indent(build_url(object.__product.base_url, key[:update_url]), 4) -%>, +<%= indent(build_url([object.__product.base_url, key[:update_url]]), 4) -%>, fetch_auth(@resource), 'application/json', { From fcb3d41dc3a0c1ae07c974290f01e6c6fe2b9658 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 22:50:05 +0000 Subject: [PATCH 18/22] tests are a real joy --- spec/provider_chef_spec.rb | 1 + spec/provider_puppet_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/spec/provider_chef_spec.rb b/spec/provider_chef_spec.rb index e5e8a8ee4e7c..11091e468075 100644 --- a/spec/provider_chef_spec.rb +++ b/spec/provider_chef_spec.rb @@ -234,6 +234,7 @@ def allow_open_real_templates allow_open 'templates/resourceref_mocks.erb' allow_open 'templates/return_if_object.erb' allow_open 'templates/transport.erb' + allow_open 'templates/puppetchef/update_props.erb' allow_open_real_tests diff --git a/spec/provider_puppet_spec.rb b/spec/provider_puppet_spec.rb index bb69ef6d01f4..65f249e768e0 100644 --- a/spec/provider_puppet_spec.rb +++ b/spec/provider_puppet_spec.rb @@ -694,6 +694,7 @@ def allow_open_real_templates allow_open 'templates/network_spec.yaml.erb' allow_open 'templates/return_if_object.erb' allow_open 'templates/transport.erb' + allow_open 'templates/puppetchef/update_props.erb' allow_open_real_tests allow_open_real_resourceref From 4d7f5ae0349582b1f0feee7fb42d8ba164754fbc Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Tue, 31 Jul 2018 23:01:32 +0000 Subject: [PATCH 19/22] rebase issues because object change --- provider/core.rb | 2 +- templates/ansible/resource.erb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/provider/core.rb b/provider/core.rb index c694065191b9..5c98fb7858cb 100644 --- a/provider/core.rb +++ b/provider/core.rb @@ -580,7 +580,7 @@ def properties_by_custom_update(properties) end def update_url(resource, url_part) - return self_link_url(resource) if url_part.nil? + return resource.self_link_url if url_part.nil? [resource.__product.base_url, url_part].flatten.join end diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index 67e59f4ca7af..f1ee9054ec83 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -274,14 +274,14 @@ def update_fields(module, request, response): difference = GcpRequest(request).difference(GcpRequest(response)) auth = GcpSession(module, <%= quote_string(prod_name) -%>) <% update_props.each do |key, props| -%> - if <%= props.map { |prop| "difference.get(#{quote_string(prop.name.underscore))})" }.join(' || ') -%>: + if <%= props.map { |prop| "difference.get(#{quote_string(prop.name.underscore)})" }.join(' || ') -%>: auth.<%= key[:update_verb].downcase -%>( ''.join([ "<%= object.__product.base_url -%>", "<%= key[:update_url].gsub('{{', '{').gsub('}}', '}') -%>" ]).format(**module.params), { -<%= lines(indent(request_properties(props), 12)) -%> +<%= lines(indent(request_properties(props, 12), 12)) -%> } ) <% end # update_props.each -%> From c6e20bc4477cee4a62640259a553553d012c3ab9 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Thu, 9 Aug 2018 17:18:10 -0700 Subject: [PATCH 20/22] trying to undo some API changes --- products/compute/api.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/products/compute/api.yaml b/products/compute/api.yaml index 3a33097fa3fd..4dd4f7d42a8a 100644 --- a/products/compute/api.yaml +++ b/products/compute/api.yaml @@ -146,6 +146,7 @@ objects: internally during updates. update_url: 'projects/{{project}}/regions/{{region}}/addresses/{{name}}/setLabels' update_verb: :POST + min_version: beta - !ruby/object:Api::Resource name: 'Autoscaler' kind: 'compute#autoscaler' @@ -899,6 +900,7 @@ objects: internally during updates. update_url: 'projects/{{project}}/regions/{{region}}/forwardingRules/{{name}}/setLabels' update_verb: :POST + min_version: beta - !ruby/object:Api::Type::Enum name: 'networkTier' description: | @@ -998,6 +1000,7 @@ objects: internally during updates. update_url: 'projects/{{project}}/global/addresses/{{name}}/setLabels' update_verb: :POST + min_version: beta - !ruby/object:Api::Type::Enum name: 'ipVersion' description: | From d4701962d21e0dbc8b326dfc6863f51ae768954d Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 10 Aug 2018 10:30:40 -0700 Subject: [PATCH 21/22] build_url problems --- provider/core.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/core.rb b/provider/core.rb index 5c98fb7858cb..e7c4d57795b8 100644 --- a/provider/core.rb +++ b/provider/core.rb @@ -580,7 +580,7 @@ def properties_by_custom_update(properties) end def update_url(resource, url_part) - return resource.self_link_url if url_part.nil? + return build_url(resource.self_link_url) if url_part.nil? [resource.__product.base_url, url_part].flatten.join end From 49115f271abff2666d06f7691c6ec90178c10fe0 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 10 Aug 2018 13:37:07 -0700 Subject: [PATCH 22/22] bad formatting on update_fields --- templates/ansible/resource.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/ansible/resource.erb b/templates/ansible/resource.erb index f1ee9054ec83..53931dff6a20 100644 --- a/templates/ansible/resource.erb +++ b/templates/ansible/resource.erb @@ -281,7 +281,7 @@ def update_fields(module, request, response): "<%= key[:update_url].gsub('{{', '{').gsub('}}', '}') -%>" ]).format(**module.params), { -<%= lines(indent(request_properties(props, 12), 12)) -%> +<%= lines(request_properties(props, 16)) -%> } ) <% end # update_props.each -%>