Kubeadm: implement a kubeadm log file that also guards kubeadm init and join commands

[neolit123]

the issue

users end up calling kubeadm init / join on the same node and we don't have good error messaging for that.

https://github.com/kubernetes/kubeadm/issues/974

the solution

the idea here, as discussed in a SIG meeting with @timothysc , @chuckha and @liztio , is to create a log file which will be used to determine if kubeadm init or kubeadm join has previously ran on this particular machine.

this change should be split into two phases.

phase 1:

assigned to: @chenyb4

• [ ] PR: https://github.com/kubernetes/kubernetes/pull/66534

phase 1 is nice to have for k8s 1.12. the idea here is just create the file and determine if init or join were used for creating it.

• create a new file called cmd/kubeadm/app/util/logfile.go.
• in that file create a couple of constants:
• KubeadmLogFolder = /var/lib/kubeadm
• KubeadmLogFile = kubeadm.log
• add methods in this .go file for writing / reading and deleting a file
• permissions for the log file and folder should be 0666
  everyone can read/write as this is just a log file, this would also solve the case where kubeadm is called with or without sudo.
• file and folder should be created when kubeadm join and kubeadm init are called
• if kubeadm init is called write kubeadm init\n to the file, else if kubeadm join is called write kubeadm join\n
• if kubeadm init or kubeadm join are called while the file exists, print an error message saying kubeadm <CMD> was already called on this machine, replace <CMD> with the command from the log file.
• the folder and file should be removed when kubeadm reset is called.

phase 2:

phase 2 can go for a future release of k8s after 1.12. the idea is to be able to log recent kubeadm calls in this file, until reset is called.

• add timestamps and all flags to the log file, for example:

2009-11-10T23:00:00Z kubeadm init --config=....
2009-11-10T23:00:00Z kubeadm config images list
...

This should be done using time.Format(), time.Parse() and the time.RFC3339 layout. For the flags we need to get them from Cobra.

• add Windows support with logfile_windows.go vs logfile_unix.go. different constants should be enough for the separate OS files.
• write whole commands and flags / arguments
• if kubeadm init or kubeadm join are called while the file exists, print an error message saying kubeadm <CMD> was already called on this machine, replace <CMD> with the command from the log file, print a timestamp, but don't include all flags.
• support filtering, removal of duplicate entries.
• document the behavior at k/website.

phase 3:

optional?

• possibly add a command to kubeadm for showing the contents of the file - e.g. kubeadm log show -N, where -N can print the last N entries.

/cc @timothysc @chuckha @liztio
/cc @kubernetes/sig-cluster-lifecycle-feature-requests

both @xlgao-zju @chenyb4 wanted to work on this.
if you guys want you can split the work on phase 1 and 2?

in the meantime this is open for comments and questions.

[neolit123]

@timothysc @chuckha @liztio
updated after the discussion today. will now suggest changes to the PR.

[timothysc]

/assign @timothysc

[neolit123]

i'm tempted to drop this idea completely at this point.
i also discovered one minor security concern with writing the log file.

the associated PR is also stale:
https://github.com/kubernetes/kubernetes/pull/66534

close?

[bart0sh]

I'm all for closing. Didn't like it from the very beginning.

[chuckha]

I agree, I don't think this is the right approach.

[fabriziopandini]

I'm in favor of closing because I don't have clear the target use case.

users end up calling kubeadm init/join on the same node

this is already monitored by preflight checks and I don't' think we should provide additional mechanism for:

• preventing mistakes when someone uses --ignore-preflight-errors=All
• giving a nice error message when someone wants to run the two commands on the same machine (because documentation explain clearly what is the correct approach for single node cluster)

[MalloZup]

i would vote for closing this issue, agreed with others. I don't think adding a lock-file in the stack provide benefits and also for user-experience, at some point we cannot prevent every non supported operation :sunflower: :smile_cat:

[neolit123]

/close

closing this as per the vote of the majority.
the related PR is closed now too:
https://github.com/kubernetes/kubernetes/pull/66534

[k8s-ci-robot]

@neolit123: Closing this issue.

In response to this:

/close

closing this as per the vote of the majority.
the related PR is closed now too:
https://github.com/kubernetes/kubernetes/pull/66534

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.