Export data as expected by import method

[stuartnelson3]

The original code was using to_hash in backup_standard. However, to_hash (http://www.rubydoc.info/gems/chef/Chef/Node#to_hash-instance_method) doesn't export the data expected from the from_hash method on Node (eventually called from http://www.rubydoc.info/gems/chef/Chef%2FKnife%2FCore%2FObjectLoader%3Aobject_from_file).

Using for_json(http://www.rubydoc.info/gems/chef/Chef%2FNode%3Afor_json) exports the attributes individually in the json, and is allowing me to load the nodes correctly from the backup.

addresses #57

[grobie]

Does this work with both Chef 11 and Chef 12? Or do we need a version switch here?

[stuartnelson3]

Chef 11 uses json_create, while Chef 12 uses from_hash. The methods are more or less identical.

[stuartnelson3]

For reference: Chef 11

def self.json_create(o)
  node = new
  node.name(o["name"])
  node.chef_environment(o["chef_environment"])
  if o.has_key?("attributes")
    node.normal_attrs = o["attributes"]
  end
  node.automatic_attrs = Mash.new(o["automatic"]) if o.has_key?("automatic")
  node.normal_attrs = Mash.new(o["normal"]) if o.has_key?("normal")
  node.default_attrs = Mash.new(o["default"]) if o.has_key?("default")
  node.override_attrs = Mash.new(o["override"]) if o.has_key?("override")

  if o.has_key?("run_list")
    node.run_list.reset!(o["run_list"])
  else
    o["recipes"].each { |r| node.recipes << r }
  end
  node
end

[leochen4891]

I ran into the same problem when exporting node objects. This PR works for me. Thanks @stuartnelson3

[kamaradclimber]

Any news on that PR? This makes the current version (0.0.12) unusable and leads user to think they are safe 🗡

[chrisgit]

I'd like to see the changes pulled in and a new gem built.

A while ago we ended up writing our own set of utilities but I'd always meant to update the knife backup gem to ensure nodes could be backed up and restored with Chef 12.

I have a couple of questions with the submitted code, the code largely duplicates backup_standard but does not use the load_object method to load the node so will not catch Net::HTTPServerException.

Would it be easier to put a hook or condition In backup_standard method?

klass.list.each do |component_name, url|
        next if component == "environments" && component_name == "_default"
        ui.msg "Backing up #{component} #{component_name}"

        component_obj = load_object(klass, component_name)

        unless component_obj
          ui.error "Could not load #{klass} #{component_name}."
          next
        end

    component_obj = klass == Chef::Node ? component_obj.for_json : component_obj.to_hash

        File.open(File.join(dir, "#{component_name}.json"), "w") do |component_file|
          component_file.print(JSON.pretty_generate(component_obj))
        end
end

Even if a new method was required to format the object in order to make the code more readable, i.e.

    component_obj = format_object_for_backup(klass, component_obj)
        File.open(File.join(dir, "#{component_name}.json"), "w") do |component_file|
          component_file.print(JSON.pretty_generate(component_obj))
        end

Then should any switches for Chef 11/12 be require it ought to be a case of updating the method.

[stuartnelson3]

Hey everyone commenting:

At this point, we at SoundCloud just rolled a gem with these changes and have been using it since. If anyone wants to improve upon this PR feel free to submit a change. As it seems clear this repo is unmaintained, I would recommend creating your own gem.

[kamaradclimber]

@stuartnelson3 could you reopen the issue? Even if you are not interested in the outcome, this is important to keep the same place to discuss the issue.

thanks

[stuartnelson3]

This is a pull request. I suggest discussion be moved to the issue here: https://github.com/mdxp/knife-backup/issues/57

[kamaradclimber]

ok thx