operator: avoid unexpected object patching

[avalluri]

On redeploying the objects operator should not unset the defaults chosen
by the API server, otherwise it results in patch diff and leads to
unnecessary update object call. This is observed both driver
statefulset and daemonset objects.

To avoid this we have two options:

1. set only specific options instead of total refresh.
2. set also the default values

We chose option 1 while setting daemonset and statefulset specification
fields, and option 2 for setting container values.

FIXES #924

[pohly]

Can you add a test for this? As it stands now, we have to be very careful about how we update the objects, and that is easy to get wrong because the effect is very subtle when done incorrectly.

Please also add a comment somewhere that removing values that were previously set by the operator but now no longer are desired needs special code.

[avalluri]

Can you add a test for this?

Testing this is a bit hard. These defaults are set by the API server, which cannot be tested in unit tests, and in E2E tests it is not possible to determine that the sub-objects updates are coming from the operator. We might introduce an event and emit that while patching an object., which could be used in the test if is expected or not.

As it stands now, we have to be very careful about how we update the objects, and that is easy to get wrong because the effect is very subtle when done incorrectly.

In fact, I guess these patch calls would not result in updating the object at the server-side, that is the reason is not caught by driver validation of unexpected update check in e2e tests.

[pohly]

Can you add a test for this?

Testing this is a bit hard. These defaults are set by the API server, which cannot be tested in unit tests, and in E2E tests it is not possible to determine that the sub-objects updates are coming from the operator. We might introduce an event and emit that while patching an object., which could be used in the test if is expected or not.

Events are not reliable. There is garbage collection for them. It shouldn't be a big problem in practice for us therefore I am not too worried that the downgrade testing is using them, but it's still better to avoid the dependency if possible.

What about introducing some metric for the operator? That then can be queried reliably - if it can be queried from outside the cluster. In those cases where we currently read metrics, we do so via the socat pod running alongside the driver pod, but that only works in testing deployments, not in production deployments, and not for the operator.

We probably need to copy some code from https://github.com/kubernetes/kubernetes/blob/master/test/e2e/storage/drivers/proxy/portproxy.go and then will have a Dial(pod, port) function which can connect from outside. Let me try to implement that...

As it stands now, we have to be very careful about how we update the objects, and that is easy to get wrong because the effect is very subtle when done incorrectly.

In fact, I guess these patch calls would not result in updating the object at the server-side, that is the reason is not caught by driver validation of unexpected update check in e2e tests.

I'm not so sure. Some of the test failures in #924 were about unexpected object changes.

[pohly]

We probably need to copy some code from https://github.com/kubernetes/kubernetes/blob/master/test/e2e/storage/drivers/proxy/portproxy.go and then will have a Dial(pod, port) function which can connect from outside. Let me try to implement that...

See #934