kubeadm: backup kubelet config for "upgrade node" and "upgrade apply"

[chendave]

• cherry pick the previous commit which is reverted by revert feature to backup kubelet config for "upgrade node", co-authored by @pacoxu

• fix invalid cross-device link error, for the issue first reported here fix failing dryrun e2e job kubeadm#2796 and the journal from here: https://storage.googleapis.com/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-kinder-patches-latest/1605265548596219904/build-log.txt
  The root cause for that error is because rename doesn't work across different mount points.
  The kubelet config file and back up directory are mounted to different file system in kinder environment.

  df /var/lib/kubelet/config.yaml | tail -n1 | awk '{print $1}'
  /dev/sda2
  df /etc/kubernetes/tmp/kubeadm-kubelet-configxxx | tail -n1 | awk '{print $1}'
  overlay
  

  Call cp instead of rename to back up the kubelet file would fix that issue.

• include the err got from PrintDryRunFile
  The error was ingored which means if anything wrong from PrintDryRunFiles, it was sliently ignored.

• backup kubelet config file for upgrade apply
  Backup kubelet config file for kubeadm upgrade apply, some code refactoring is done to de-dup some redundant code logic.

test result with kinder: https://gist.github.com/chendave/64f7825cf7faa7f30774fe6c48053c21

/kind feature

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kubelet config file will be backed up to `/etc/kubernetes/tmp/` folder with `kubeadm-kubelet-config` append with a random suffix as the filename 

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[chendave]

/hold
need more self-testing, but I believe this pr will fix the issue we found.

/cc @neolit123 @pacoxu

[chendave]

• here is the test result with kinder: https://gist.github.com/chendave/64f7825cf7faa7f30774fe6c48053c21
• update the pr with testcases created.

/hold cancel
this is ready for review now.

[chendave]

/test pull-kubernetes-e2e-gce

[pacoxu]

I am working on some old PR follow-ups this week and will update some KEPs in this week and next. I will get back to this later this week or next then.
/assign

[chendave]

need a rebase since #114719 was merged.

[pacoxu]

Should we handle the return err here?

err := upgrade.WriteKubeletConfigFiles(cfg, data.PatchesDir(), dryRun, data.OutputWriter())

[chendave]

yes! addressed.

[chendave]

@pacoxu pls see https://github.com/kubernetes/kubernetes/compare/f6b04fe5d4e168067d978440af4464134f57caa4..484d0d542e3adbcbda08a48236daabcfe1649504 for the diff.

added the check for privileged user so that the test can only run with the root user. Your comments leads me to think about the regression we have: #114824 👍

[pacoxu]

/priority important-soon
/triage accepted

https://github.com/kubernetes/kubernetes/compare/f6b04fe5d4e168067d978440af4464134f57caa4..484d0d542e3adbcbda08a48236daabcfe1649504 for the diff.

added the check for privileged user so that the test can only run with the root user. Your comments leads me to think about the regression we have: #114824 👍

I am not sure if this needs to be fixed or not, and is this the best practice to skip it like this?

Overall
/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 2bb78489bb0588d545d9c542de3d6353e3440aeb

[chendave]

I am not sure if this needs to be fixed or not, and is this the best practice to skip it like this?

my understanding is that upgrade needs the user to be root, so it is non-sense to run those test if the user is not root, besides, I was thinking to create the dir which every user has the permission to write on all the arches & os, but I don't think its possible.

[neolit123]

/hold
/approve

please test with kinder and the related e2e test workflow before merging / removing hold.
docs on how to use kinder can be found in the k/kubeadm repo

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chendave, neolit123

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:

• cmd/kubeadm/OWNERS [neolit123]

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

[neolit123]

actually i realize it's not so simple to test uncommited code with kinder....that is because the tool is mostly designed for testing artifacts that are already build from k/k commits in CI.

alternatively just remove the hold watch the e2e test signal in the next 2 days and revert / update if needed:
https://testgrid.k8s.io/sig-cluster-lifecycle-kubeadm

[chendave]

@neolit123 I tested it locally, and here is the link of the result I pasted in the desc,

test result with kinder: https://gist.github.com/chendave/64f7825cf7faa7f30774fe6c48053c21

[chendave]

/hold cancel

let's see if the CI job is happy.

[pacoxu]

/retest

[pacoxu]

https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-kinder-patches-latest/1622937946548080640
This passed after merge.