Do not deepcopy, instance can be nil

[alenkacz]

No description provided.

[nfnt]

This isn't actually needed, DeepCopy() handles the case that instance is nil:

func (in *TestType) DeepCopy() *TestType {
	if in == nil {
		return nil
	}
	out := new(TestType)
	in.DeepCopyInto(out)
	return out
}

(neat Go trick btw)

[alenkacz]

oh, goland was complaining... I would still probably prefer to merge this in even though it's not fixing anything. Might be handy for people (and IDEs) who don't know deepcopy implementation and might freak them out as it did freak out me :D

[nfnt]

But instance might still be nil after calling getInstance when it isn't found.
Hmm, looks like the existing code is wrong here, becase the apierrors.IsNotFound isn't necessary or should be in a different place.

[alenkacz]

omg yeah, you're right... I am starting to feel like we should get rid of these special getXX methods returning nil because it's just confusing, when it returns something different than k8s client. WDYT?

[nfnt]

I'm fine with having helper functions.
Though, the code is actually right: getInstance doesn't do what's documented, it doesn't return nil, nil if an instance isn't found. Let's remove the wrong documentation comment.

Though there is similar code in pkg/kudoctl/util/kudo that returns nil, nil in that case. We should at least be consistent here, so that manager and CLI can share more code. #1002 is supposed to go in this direction and provide a KUDO API that could be used by the manager, the CLI and probably external projects as well.

[alenkacz]

Oh I was questioning the fact of returning nil vs not found error - I think it's confusing if 10% of code does nil, the rest err, also you need to check for nil etc. anyway so the code does not get that much nicer. But then I see that I already changed it here #850 just did not update the docs :D