Fix node local storage

[rata]

There are two commits, with messages explaining them in detail. I'll copy paste one, the most important one that actually fixes the issue, here:

The rkt pod is exposed with /mnt/ volume1 and in this PR:

    https://github.com/kinvolk/lokomotive-kubernetes/pull/48/files#diff-b6c7caf796cd86bdcdd936319b1793a1R152

the location where the RAID 0 array is mounted was changed from /mnt
to /mnt/<some-dir>. This breaks using local volumes in pods, as it
seems the mount is not visible for the rkt container or the kubelet
running in rkt.

The investigation of the root cause of this issue, to understand why
mounts inside mnt (like in /mnt/node-local-storage) can't be used by
pods currently and what needs to be changed for that, is left as a for
future patches and issue #73 was created to investigate this.

This patch just makes the mount location back to /mnt for all the cases
(setup_raid, setup_raid_*) so it works on all cases. However, this
imposes one limitation: setup_raid_hdd and setup_raid_ssd are mutually
exclusive now.

This limitation is not limiting something that was working in master, in
fact setup_raid_hdd and setup_raid_ssd (when setup_raid_ssd_fs is set)
were completely broken since they were merged and pods never wrote to
those volumes, for the very same reason issue #73 states: mounts insde
/mnt are not visible to pods.

Therefore, this patches fixes node local storage while making those
options exclusive (only one can be set), and this is not a big problem
as those options never really worked.

Some alternative fixes were considered, like changing the path that is
exposed to rkt container to be /mnt/node-local-storage or
/mnt/node-hdd-local-storage, according to what option was used
(setup_raid, setup_raid_hdd, and exposing both if both are set), but
that was messy without any good reason (it is better to tackle #73
before doing something that ofuscated). So, we decided for this more
simpler approach.

This patch is just a minimal fix, "a revert" to mount in /mnt/ again,
to make this work again on master.

As a side effect of this PR, too, another issue to reconsider if we need
so many setup_raid_* flags was created
(#74) and to even
consider a totally different approach than the current bash script
before it gets out of control: #75

[invidian]

LGTM, but please create follow up issues as we discussed

[surajssd]

LGTM

[pothos]

SSD and HDD RAID are currently not exclusive: https://github.com/kinvolk/lokomotive-kubernetes/blob/master/packet/flatcar-linux/kubernetes/workers/cl/worker.yaml.tmpl#L216 (I would remove the comment and turn the second if into an else-if.)

At least, please update the docs to say that they are exclusive.

Maybe we can we validate this in terraform and throw an error as early as possible?

[rata]

@pothos oh, good point! It is exclusive with setup_raid but those others are not exclusive. Thanks for noticing!

I'll follow your recommendation and put and if-else there, document them as exclusive and create two issues: one to address the root cause and make sure we understand why mounts are not seen by the kubelet when done inside and another issue to either remove those options (not sure they serve any useful purpose, would like to double check that because the code is messy already) or fix them so both can be used at the same time (right not, sine they were added, it was not working as expected).

[rata]

@pothos PTAL :)

Will also update the PR description with these changes.

[pothos]

I'm ok with this and hope it stays that way for some time but of course we can change it again later if needed. The other approach which forwards the subdirectories to the kubelet is maybe more generic (and the terraform template can make use of the variables to only set the option that is needed).

[surajssd]

I am not really sure, why this change was introduced?

With this code now we can only set RAID of type HDD or SSD. This change has added another regression!

[rata]

Because mounting hdd and sdd both on /mnt is not possible, and the original patch was doing that (if you configure both to have an fs. This is explained in the updated PR description :-)

And those changes never worked (new setup_raid_* flags), they never wrote to those devices, as things mounted under /mnt are not visible to pods. I've created #74 to double check if we really need this and also created #73 to tackle the root cause and even proposed a fix there (if better fixes can't be found), see the comment that links to ba80265

However, as far as I understand those never worked and my reasoning was this: as those variables never worked, introduced for a few commits a backwards incompatible change and I'm not really sure we need them, let's just revert to the previous working situation and if we need to introduce them and do a backwards incompatible change, let's just do it. But it would be great if we can avoid using these, as the code is getting out of control (also created #75 to suggest a path to keep this under control) and introducing more bugs than solutions (as just created bugs but never really worked :-D).

If we later need to do the backwards incompatible change, let's do them (as we tried to do it when we first introduced these vars and change mounting from /mnt to /mnt/node-local-storage). But let's do them making sure everything works and, if possible, trying to make the change as simple as possible and not generate much tech debt :)