Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] ensure non-empty $HOME in deployments #1337

Closed
Privatecoder opened this issue Nov 3, 2022 · 15 comments · Fixed by #1381 or minio/console#2565
Closed

[bug] ensure non-empty $HOME in deployments #1337

Privatecoder opened this issue Nov 3, 2022 · 15 comments · Fixed by #1381 or minio/console#2565
Assignees

Comments

@Privatecoder
Copy link

Privatecoder commented Nov 3, 2022

Expected Behavior

Always ensure non-empty $HOME in deployments

Current Behavior

  • $HOME in minio-operator and console-pods is empty when using crun as containerd-runtime, i.e. echo $HOME => empty.
  • The operator called pod with the minio/console-image therefore gets stuck in crash-loop after installing with an error-msg console: <ERROR> Unable to get mcConfigDir. exit status 2..
  • All tenant-pods created successively crash as well due to the same issue.
  • Other containers, nginx for example, have env-vars set correctly, i.e. not empty echo $HOME => /root.

Possible Solution

Ensure $HOME is not empyty

Steps to Reproduce (for bugs)

Deploy minio-operator through krew or helm on a K8s v1.24.7 cluster with containerd 1.6.9 / crun 1.6

Your Environment

  • minio-operator (tested) 4.4.28, 4.5.1, 4.5.2, 4.5.3, 4.5.4
  • kubernetes v1.24.7
  • containerd 1.6.9 with crun 1.6
@Privatecoder Privatecoder changed the title required env $HOME not set in pods after install via minio operator through krew or helm / console crashing required env $HOME not set in pods after installing minio operator through krew or helm / console crashing Nov 3, 2022
@Privatecoder Privatecoder changed the title required env $HOME not set in pods after installing minio operator through krew or helm / console crashing ensure env $HOME is set in pods after installing minio operator through krew or helm Nov 3, 2022
@Privatecoder
Copy link
Author

Privatecoder commented Nov 4, 2022

This might be an issue with cruncrun/#108 and crun/#599 – which both have been closed however it still needs fixing on the other side: podman/#9378.

I will do some more testing tomorrow but if crun indeed is the cause of the issue MinIO should ensure a non-empty $HOME if / as it requires it.

@Privatecoder Privatecoder changed the title ensure env $HOME is set in pods after installing minio operator through krew or helm [bug ]ensure $HOME in deployments Nov 4, 2022
@Privatecoder Privatecoder changed the title [bug ]ensure $HOME in deployments [bug] ensure non-empty $HOME in deployments Nov 4, 2022
@Privatecoder
Copy link
Author

Privatecoder commented Nov 5, 2022

With runc as as containerd runtime $HOME is present so this indeed is an issue with crun however MinIO should still ensure its presence.

I created a new runc RuntimeClass and modified the operators helm-chart to enable passing runtimeClassName and work around the issue.

Cheers to @dnskr and his PR.

@Privatecoder
Copy link
Author

Privatecoder commented Nov 5, 2022

This does not set the runtimeClassName of a tenants minio/minio container created through the UI however so it only works for the operator and console deployments deployed by the chart and tenants added manually.

@pjuarezd pjuarezd self-assigned this Nov 15, 2022
@dvaldivia
Copy link
Collaborator

will making runtimeClassName configurable on Tenant fix this problem?

@dvaldivia dvaldivia assigned allanrogerr and unassigned pjuarezd Dec 21, 2022
@dvaldivia dvaldivia removed the triage label Dec 21, 2022
@allanrogerr
Copy link
Contributor

@Privatecoder Please confirm @dvaldivia 's proposal above

@Privatecoder
Copy link
Author

@allanrogerr @dvaldivia

this will probably work however it would still be a workaround as changing the runtime class for a specific deployment is not always an option.

A fix would require to ensure a non-empty $HOME env

@allanrogerr
Copy link
Contributor

I'm not sure that I'm following. The helm charts for tenant already contain runtimeClassName.

  1. Using runtimeClassName: runc allows the tenant nodes to come online successfully.
  2. Switching to runtimeClassName: crun causes the tenant pods the throw the error <ERROR> Unable to get mcConfigDir. exit status 2.

This looks like a crun issue still. The $HOME environment variable was consistently populated when doing this test - it never became empty during the above test. I'm investigating a bit further.

Could you clarify if you prefer if minio pods don't start up if there is no $HOME environment variable set? Ideally all users have $HOME set.

Also based on the conversation up to now we have these below options. I'm also looking into these now

  • The helm charts for operator/console do not yet contain runtimeClassName. This can be added.
  • We can also add the ability to set the runtimeClassName of a tenant in the UI.

@Privatecoder
Copy link
Author

Privatecoder commented Jan 3, 2023

Following my post here I found, that the issue was fixed for podman like this.

So This looks like a crun issue still. is obviously the case but we should still make sure that $HOME is never empty anyway.

Adding a configurable runtimeClassName for the Operator and in the UI for the tenants will allow users to workaround the issue however I'd prefer a solid fix if this would be a choose any of the two options decision.

  • The helm charts for operator/console do not yet contain runtimeClassName. This can be added.
  • We can also add the ability to set the runtimeClassName of a tenant in the UI.

Ideally we implement these two and additionally add a fix like it has been implemented for podman

@dvaldivia
Copy link
Collaborator

@Privatecoder isn't this missing $HOME variable something that can be set on the minio-operator and console deployment on a per-need basis?

@Privatecoder
Copy link
Author

@dvaldivia I came across this issue when I tried to deploy MinIO to a new cluster which was using crun as containerd-runtime and therefore crashed with the mentioned error-message.

As I could not figure out what the issue was initially I was glad to solve this eventually with some help.

Digging into this I found other threads such as the – also mentioned – podman-issue-thread.

After fixing the operator and console by making the runtimeClassName configurable I ended up with non-working tenants for the very same reason..(of course one could go ahead and fix this manually all the time).

However & to sum it up:

Imho one would expect to be able to deploy a chart without manually fixing env-vars (other deployments have $HOME set in crun environments) or changing the runtimeClass.

I also think that since this might be an easy fix, MinIO should incorporate something similar to the fix here (or others).

@dvaldivia
Copy link
Collaborator

we were able to replicate the issue, @allanrogerr will send a fix for tenants in the particular case of crun runtime, but as for the deployments for minio-operator and console on the minio-operator namespace I'm wondering if it's worth adding HOME to the deployments themselves or if this is something the administrators can fix when they are installing the Operator

@Privatecoder
Copy link
Author

Like I wrote: If the minio-operator and console rely on it than we should at least name it in the docs however I would suggest fixing it like for the tenants.

@dvaldivia
Copy link
Collaborator

sure, let's document this @allanrogerr

@cniackz
Copy link
Contributor

cniackz commented Jan 5, 2023

It will be fixed in #1381

allanrogerr added a commit to allanrogerr/operator that referenced this issue Jan 5, 2023
Add a note for tenants on spec.pools.runtimeClassName as requested on minio#1337
@allanrogerr
Copy link
Contributor

Added to UI in minio/console#2626
All outstanding requests addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants