openshift / machine-api-operator

Machine API operator
Apache License 2.0
164 stars 203 forks source link

The machine-api-operator overwrites taints written by either API or oc/kubectl commands #144

Closed yuqi-zhang closed 5 years ago

yuqi-zhang commented 5 years ago

With a cluster brought up with the 4.0 installer, it seems that the only way to apply a taint is to kubectl edit machine ... and add the taint much like https://github.com/openshift/machine-api-operator/blob/master/install/0000_50_machine-api-operator_02_machine.crd.yaml#L43 . If I try to do so via oc/kubctl or a direct Taint API call, it would get overwritten.

Is this by design moving forward or am I doing something wrong? For reference, in RHCOS we would like nodes to be tainted if someone SSH'es into them, and have admins be able to un-taint the machines via. oc if necessary.

smarterclayton commented 5 years ago

This is a bug. Tainting should work.

smarterclayton commented 5 years ago

This blocks several kube conformance tests:

https://openshift-gce-devel.appspot.com/build/origin-ci-test/logs/release-openshift-origin-installer-e2e-aws-all-4.0/3/#openshift-tests-sig-cli-kubectl-client-k8sio-kubectl-taint-serial-should-remove-all-the-taints-with-the-same-key-off-a-node-suiteopenshiftconformanceserial-suitek8s

wongma7 commented 5 years ago

"// Taints are to be an authoritative list on the machine spec per cluster-api comments:" https://github.com/openshift/machine-api-operator/blob/cbd9a1d62ba4765397e6317724b35d688f67d9fa/cmd/nodelink-controller/main.go#L396

wongma7 commented 5 years ago

here is the cluster-api comment that the above comment is referring to: https://github.com/kubernetes-sigs/cluster-api/blob/5a27d984f9af41aa4f952e7c36a11bfbfb042b83/pkg/apis/cluster/v1alpha1/machine_types.go#L56 "// The full, authoritative list of taints to apply to the corresponding // Node. This list will overwrite any modifications made to the Node on // an ongoing basis."

so to fix this it may require some help from upstream cluster-api to decide whether machine.spec.Taints ought to be authoritative

ashcrow commented 5 years ago

This also blocks the taint process for accessing RHCOS (which is what @yuqi-zhang is working on)

yuqi-zhang commented 5 years ago

Can someone who maintains this repo comment on this? Would this be an applicable fix? It would help unblock work in the machine-config-operator. Thanks!

ashcrow commented 5 years ago

Can someone who maintains this repo comment on this? Would this be an applicable fix? It would help unblock work in the machine-config-operator. Thanks!

/cc @enxebre @paulfantom

ashcrow commented 5 years ago

Can we get a response on this?

enxebre commented 5 years ago

Thanks for reporting this. We just made machine taints additive rather than authoritative https://github.com/openshift/machine-api-operator/pull/154 in a follow up we plan to possibly use the machine taints for reflecting only the initial state of the node https://github.com/kubernetes-sigs/cluster-api/pull/651

enxebre commented 5 years ago

closed by https://github.com/openshift/machine-api-operator/issues/144#issuecomment-451214530 Please re open if relevant