Bug 1735502: compare solely md5 hash for config change

[alexanderConstantinescu]

Hi

This is a PR issued for resolving Bug 1735502. It seems the sync pod compares node config using the md5sum output, which included file names. This will thus lead to diffs even though the md5 sum is equal. The consequence of this is: killing the kubelet when it should not, which might have several unwanted consequences, but surely in some cases, has this: the network is not properly setup as the kubelet and required CNI plugins are killed unexpectedly during their network setup.

The fix uses this answer for only getting the hash of md5sum

Please let me know in case this needs to be ported to 4.X version as well. I didn't find this file in release-4.X, but let me know in case I missed something.

/assign @squeed @danwinship

[openshift-ci-robot]

@alexanderConstantinescu: This pull request references Bugzilla bug 1735502, which is invalid:

• expected dependent Bugzilla bug 1748032 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is POST instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1735502: compare solely md5sum for config change

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[alexanderConstantinescu]

/assign @patrickdillon

[openshift-ci-robot]

@alexanderConstantinescu: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-crio	d0a5fa3	link	/test e2e-gcp-crio

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[patrickdillon]

Interesting. To help me understand, do you have an example of how including the filename would create a diff?

[alexanderConstantinescu]

Interesting. To help me understand, do you have an example of how including the filename would create a diff?

Hi

Sure. Currently, on line 63 we do:

md5sum /etc/origin/node/node-config.yaml > /tmp/.old

This writes the following to /tmp/.old (as an example):

$ cat /tmp/.old
d428b61590a76bbcd5ad07912d074f6f  /etc/origin/node/node-config.yaml

Next on line 143, we do:

md5sum /etc/origin/node/tmp/node-config.yaml > /tmp/.new

Which results in the following (as an example):

$ cat /tmp/.new
d428b61590a76bbcd5ad07912d074f6f  /etc/origin/node/tmp/node-config.yaml

This means that the comparison on line 152:

if [[ "$( cat /tmp/.old )" != "$( cat /tmp/.new )" ]]; then

will never be equal, even if the hash is - due to the original file location being different.

[tsmetana]

Hi. I'm sorry to nag but the bug this PR is supposed to fix is rather urgent. Could you please review and eventually merge?
Thanks.

[patrickdillon]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexanderConstantinescu, patrickdillon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [patrickdillon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[patrickdillon]

/skip

[patrickdillon]

/bugzilla refresh

[openshift-ci-robot]

@patrickdillon: This pull request references Bugzilla bug 1735502, which is invalid:

• expected dependent Bugzilla bug 1748032 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is POST instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[alexanderConstantinescu]

/bugzilla refresh

[openshift-ci-robot]

@alexanderConstantinescu: This pull request references Bugzilla bug 1735502, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci-robot]

@alexanderConstantinescu: All pull requests linked via external trackers have merged. Bugzilla bug 1735502 has been moved to the MODIFIED state.

In response to this:

Bug 1735502: compare solely md5 hash for config change

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mtnbikenc]

/cherrypick release-3.10

[openshift-cherrypick-robot]

@mtnbikenc: #11878 failed to apply on top of branch "release-3.10":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	roles/openshift_node_group/files/sync.yaml
Falling back to patching base and 3-way merge...
Auto-merging roles/openshift_node_group/files/sync.yaml
CONFLICT (content): Merge conflict in roles/openshift_node_group/files/sync.yaml
Patch failed at 0001 Bug 1735502: compare solely md5sum for config change, not default md5sum output which includes file name

In response to this:

/cherrypick release-3.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.