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

Add /etc/hosts file back #384

Merged
merged 1 commit into from
Jul 7, 2021
Merged

Add /etc/hosts file back #384

merged 1 commit into from
Jul 7, 2021

Conversation

mudler
Copy link
Contributor

@mudler mudler commented Jul 6, 2021

While it fixes things for from scratch installs, it breaks many things:

  • we should generate an /etc/hosts file during boot, ideally in
    cloud-init phase, but that looks cumbersome as we can just relay on
    what's the default
  • going to that route, and then excluding /proc, /sys, /run and /tmp
    requires further work on the bootstrap part, currently tests are not
    passing (see Exclude /proc and /sys #366)

It sounds easier instead to have a separate package for it just for
creating derivatives "from scratch" - in that way we don't interfere
with the standard system/cos which is supposed to run on ISO and other
medium too, where such folders are mandatory.

This addition adds a system/cos-container to just provide a separate package
with /proc, /run, /sys, /dev, /tmp stripped which is meant to be used for
from scratch scenarios.

Supersedes: #366

Signed-off-by: Ettore Di Giacinto [email protected]

@mudler mudler mentioned this pull request Jul 6, 2021
@davidcassany
Copy link
Contributor

davidcassany commented Jul 6, 2021

mmm while I am totally in favor to get back /etc/hosts I believe the other folders still need to be cleared, otherwise is a bad design. I believe some failures are because the folder is excluded, but not its contents.

After second look I believe #366 mostly fails because it should have been

- ^/tmp/.*
- ^/proc/.*
- ^/sys/.*
- ^/run/.*
- ^/dev/.*

instead of

- ^/tmp
- ^/proc
- ^/sys
- ^/run
- ^/dev

some of these folders, if not all, are part of the readonly image, so at boot systemd can't create mountpoint.

@mudler
Copy link
Contributor Author

mudler commented Jul 6, 2021

- ^/tmp/.*
- ^/proc/.*
- ^/sys/.*
- ^/run/.*
- ^/dev/.*

Good point, but for the purpose of #366 excluding its content is not enough, luet will try to lchown directories too if they are present in the package. Let's say you do a mkdir foo, and chown it to something. Luet will try to reconstruct that on the host so even if empty that would hit lchown errors again.

That being said, I can try to add nevertheless

- ^/tmp/.*
- ^/proc/.*
- ^/sys/.*
- ^/run/.*
- ^/dev/.*

to the excludes of system/cos, as it sounds a good thing to add anyway 👍

@mudler mudler force-pushed the add_hosts_back branch 2 times, most recently from 07ec3c4 to 6b56092 Compare July 6, 2021 15:46
@mudler
Copy link
Contributor Author

mudler commented Jul 6, 2021

@davidcassany added in 6b56092 👍

@davidcassany
Copy link
Contributor

to the excludes of system/cos, as it sounds a good thing to add anyway 👍

I am not sure about /dev folder though... Probably I'd start without this one.

@mudler
Copy link
Contributor Author

mudler commented Jul 6, 2021

to the excludes of system/cos, as it sounds a good thing to add anyway +1

I am not sure about /dev folder though... Probably I'd start without this one.

Sure! I don't think anyhow we are recreating any of them with mknod, so should be safe

@mudler
Copy link
Contributor Author

mudler commented Jul 7, 2021

ok, something is not ok by excluding those files, I'll split that in a separate PR

While it fixes things for from scratch installs, it breaks many things:

- we should generate an /etc/hosts file during boot, ideally in
  cloud-init phase, but that looks cumbersome as we can just relay on
what's the default
- going to that route, and then excluding /proc, /sys, /run and /tmp
  requires further work on the bootstrap part, currently tests are not
passing (see #366)

It sounds easier instead to have a separate package for it just for
creating derivatives "from scratch" - in that way we don't interfere
with the standard system/cos which is supposed to run on ISO and other
medium too, where such folders are mandatory.

This addition adds a system/cos-container to just provide a separate package
with /proc, /run, /sys, /dev, /tmp stripped which is meant to be used for
from scratch scenarios.

Supersedes: #366

Signed-off-by: Ettore Di Giacinto <[email protected]>
@mudler
Copy link
Contributor Author

mudler commented Jul 7, 2021

All required tests are passing, merging

@mudler mudler merged commit 7343aad into master Jul 7, 2021
@mudler mudler deleted the add_hosts_back branch July 7, 2021 10:01
frelon pushed a commit to frelon/elemental-toolkit that referenced this pull request May 12, 2023
)

This commit instead of swallowing viper errors when loading config
files it reports back the error for build commands. This is the
same criteria as it is for the run time commands.

Signed-off-by: David Cassany <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants