kubectl label command crashes when a resource has no labels

[kazegusuri]

$ cat redis-master.json
{
  "id": "redis-master",
  "kind": "Pod",
  "apiVersion": "v1beta1",
  "desiredState": {
    "manifest": {
      "version": "v1beta1",
      "id": "redis-master",
      "containers": [{
        "name": "master",
        "image": "dockerfile/redis",
        "cpu": 100,
        "ports": [{
          "containerPort": 6379,
          "hostPort": 6379
        }]
      }]
    }
  }
}
$ kubectl create -f redis-master.json
$ kubectl get po
POD                 IP                  CONTAINER(S)        IMAGE(S)            HOST                        LABELS              STATUS
redis-master       10.244.23.2         master              dockerfile/redis    172.17.8.103/172.17.8.103   <none>              Running

$ kubectl label po redis-master name=bar 
# crash

[satnam6502]

I wonder if api.ObjectMetaFor should make sure it returns an empty map rather than nil and requiring this check for its callers?

[bgrant0607]

@brendandburns

[kazegusuri]

Modified api.ObjectMetaFor to returns an empty map.

[satnam6502]

Someone should check to ensure that this is a good change i.e. for api.ObjectMetaFor to return an empty map for empty labels rather than nil. @brendandburns or @bgrant0607 can probably make a pronouncement.

[roberthbailey]

@jlowdermilk reviewed ed9761f which originally added kubectl label and might also have an opinion.

[j3ffml]

If it's any guide, the ObjectMeta api object specifies labels as an omitempty field. I suspect there may be other code which assumes that objectmeta returns nil labels if the object has no labels. I recommend you just keep the original change in label.go.

[smarterclayton]

I'm uncomfortable with this - nil labels and nil annotations is a valid go-ism - making it here doesn't make this easier to consume.

[smarterclayton]

I would prefer clients simply guard against labels being nil.

[brendandburns]

Yeah, I think I'd prefer nil checks in the client also... @kazegusuri thanks for the PR and sorry for the run around... If you revert back to your original nil checks, I'll happily merge this PR.

Thanks again.

[kazegusuri]

OK, thank you for review. I reverted the second commit.

[brendandburns]

LGTM. Merging.

(sorry for the delay!)