knife node from file fails with NoMethodError exception for undefined method 'save'

It looks like I was missing some required things in my JSON...

Before (didn't work):

{
  "normal": {
    "sc_backupServer": {
      "syncTo": "some.repo.standingcloud.net"
    }
  },
  "name": "internal.repo.standingcloud.net",
  "chef_environment": "_default",
  "run_list": [
    "role[sc_repoServer]",
    "role[sc_backupServer]"
  ],
  "chef_type": "node"
}

After (does work):

{
  "normal": {
    "sc_backupServer": {
      "syncTo": "some.repo.standingcloud.net"
    }
  },
  "name": "internal.repo.standingcloud.net",
  "override": {
  },
  "default": {
  },
  "json_class": "Chef::Node",
  "automatic": {
  },
  "chef_environment": "_default",
  "run_list": [
    "role[sc_repoServer]",
    "role[sc_backupServer]"
  ],
  "chef_type": "node"
}

Thanks

Yeah, you need to pass this line, else it will be treated as normal hash
"json_class": "Chef::Node",

Note that the output of knife node show NODENAME -Fj does not include the necessary "json_class" attribute. This is counter-intuitive for those used to unix tools that work together:

knife node show NODENAME -Fj > node.json
# Edit node name and perhaps some other data
knife node from file node.json
# The above will fail

I got bit by this too. The real bug here is that knife node show -Fj doesn't output the json_class attr. knife environment show -Fj does, for example.

As Steven says, the problem here is that knife node show NODENAME -Fj isn't consistent with the other commands and it doesn't put the json_class attrib to the json.

I added this json_class attrib to the node_presenter and now you can export as json and load again from the file with no erros:

knife node show $NODENAME -Fj > nodes/$NODENAME.json
knife node from file nodes/$NODENAME.json

I create a pull request with the fix at Github.

I was trying to create some tests but I didn't know how well to test it. I searched for tests for a similar features (export, import with json, check for json attribs, etc) and I didn't found any that helps me.

I'll keep trying anyway.

This should be set to Resolved/Fixed for code review. We'll close the ticket when it is merged to a the Opscode git repository on github.

Oops, sorry, I think I mistaken the button :-/

Can we instead patch for not requiring json_class?

json_class is unique to the json gem and we haven't been particularly fond of it historically.

We already know what type of object we're importing; when you run 'knife node from file' it will always be a Chef::Node object.

Hummm... I didn't know json_class wasn't really require by Chef. I found it weird, but I asumed it was require for some reason.

Your solution seems better approach indeed. I'll look into it.

Thanks for the tip

I was cheking this but I found a problem. What uses Chef to convert the json file into Chef object is the JSON.parse method, which has a important bug which was closed the last month.

I uupdated the gem and seems that it works with Hash alike objects, but it still crash with the Chef::Node and so on.

The cleaner approach I had seen was to pass a object_class to the method Chef::JSONCompat.from_json:

Chef::JSONCompat.from_json(IO.read(filename), {:object_class => @klass})

But this will get a fatal crash. It won't crash though if insteaf of @klass you pass Hash alike class such as Mash, but oviously it doesn't work later in the code.

I'm trying something else (like using directly the Node method json_create), but still no luck.

I've updated the pull-request with a new solution that create the proper object Chef::Node from a json file even if the file doesn't contain the json_class.

I was testing it also with roles and enviroments to be sure it doesn't break them and (for me) it's working fine.

I've just realized that this solution has a side effect on environments:

$ knife environment from file environments/production.json
ERROR: knife encountered an unexpected error
This may be a bug in the 'environment from file' knife command or plugin
Please collect the output of this command with the `-VV` option before filing a bug report.
Exception: NoMethodError: undefined method `[]' for production:Chef::Environment

This works fine with nodes and roles, but not with environments, which doesn't implement [], so the jason_create method doesn't work.

But I think this is a bug. I'll try to fix it and see check that all the knife commands still work.

After some testing I discover that the problem wasn't with the environment. but with the files with the json_class defined.

So this solves the problem with the files without json_class but makes it fail with the files with json_class. Well, actually data_bags doesn't like to much this change...

Probably would be a good idea to make knife not to write the json_class with the -Fj option, but there is probably people out there who has already roles and environment saved with that attribute.

The problem is that original JSON.parse ignore the parameter we pass {:object_class => Hash} if there is a json_class inside the file.

I'll keep looking for a clean solution but if anyone have a idea, I all eyes

Ok, finally I got this working with all the possibilities.
I tested with all the knife's command that support from file (node, role, environment and data bag) and for all the cases I tested with and without the json_class attribute.

Even with the Ruby's bug that I mentioned earlier have been fixed, the JSON.parse method doesn't behave as it should. At least, for me, the expected behave will be to use the class I pass as argument instead of the json_class defined inside the file.
If I don't pass any :object_class, use the json_class from the file, but if I pass it probably is because I want to use it instead.

Anyway, as long as the JSON.parse ignores the parameter :object_class if the json_class is defined inside the file, I did rename that json_class before calling the Chef::JSONCompat.from_json (which call JSON.parse).
This is a bit hacky, but after a lot of trying and thinking, this was the less ugly and intrusive way that I found.

when /\.(js|json)$/
-            Chef::JSONCompat.from_json(IO.read(filename))
+            #NOTE: This should use {:object_class => @klass} but there is a
+            # Ruby bug (https://bugs.ruby-lang.org/issues/6304) that makes it
+            # crash so this is a workaround.
+            f = IO.read(filename)
+            f.gsub!('json_class','_json_class')
+            r = Chef::JSONCompat.from_json(f, {:object_class => Hash})
+            # Chef::DataBagItem doesn't work well with the json_create method
+            if @klass == Chef::DataBagItem
+              r
+            else
+              @klass.json_create(r)
+            end
           when /\.rb$/

It can be changed however once the Ruby JSON.parse take in account the parameter :object_class.

I also left the old way for the Chef::DataBagItem class because it wasn't working with it and Knife don't add the json_class to the data_bags anyway.

The changes are updated at the pull-request.

I've added some tests for the Chef::Knife::Core::ObjectLoader which hasn't any and was the one I had to modify.

Now things like the json_class can be tested and is easier to change. The tests pass with the new changes and they fail (as it is expected) without those new changes.

The tests are for loading Chef::Node, Chef::Role or Chef::Environment from a file.

The pull-request has been updated with the tests.

Juanje,

This looks pretty good, except that we're concerned with the .gsub call is a little indiscriminate and dirty. We'd like to avoid this. The best case scenario here would be switching over to Yajl, if you're interested in undertaking that. You've gotten pretty familiar with the relevant subject matter so I think you're an expert now. What do you think?

Yeah, it IS indiscriminate and very dirty. I was trying a lot of things before and this was the less uglier I found, but Yajl sounds like a much better approach

It would be much much easier if the JSON method hadn't those bugs... That's a pity.

I'd love to undertaking it I starts to get feelings for the Knife's code :-P
I'll look later into this. With the tests and the previous research, I expect to have it soon.

Thanks

Well, it took a bit more than I expected because I wasn't sure if the change produced a regresion with the bug CHEF-1292. Looking the Yajl documentation seemed so and I hadn't a test for that bug.

I created a test (a bit ugly, but I didn't see a better way to thest a json with 1000 nested values and more...) for that bug and seems that is working fine with the change to Yajl. No regresion.

I've rebased the last changes from the master (which has changes on the files I was modifing) and I've updated the pull-request. The result is simpler and nicer now.

Merged to master. Thanks Juanje.