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

Nomad config validate does not check for missing host_volume directories and daemon fails to start #16968

Open
maxadamo opened this issue Apr 23, 2023 · 8 comments · Fixed by #17393
Labels
good first issue stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/config type/enhancement

Comments

@maxadamo
Copy link

maxadamo commented Apr 23, 2023

Proposal

nomad config validate <config_file> does not detect missing files/directories for the host_volume parameter and Nomad fails to start

Use-cases

if a directory/file is missing, nomad won't start.

Attempted Solutions

With Puppet we created a custom config validation, which checks that the paths are valid on the system:
voxpupuli/puppet-nomad#85

but I'd expect this same functionality to be available on nomad config validate

@tgross
Copy link
Member

tgross commented May 15, 2023

Hi @maxadamo! I just verified that the IsValidConfig doesn't just do a static config check and it does things like check the serf keyfile config exists if provided. So this feature totally makes sense to add for host volumes and probably other options as well.

I'm going to mark this for roadmapping but also mark it as a good first issue for community contributions.

@tgross tgross added stage/accepted Confirmed, and intend to work on. No timeline committment though. good first issue labels May 15, 2023
@dttung2905
Copy link
Contributor

Hi team,

I would like to attempt to fix this issue. I just added a PR above 🙏

@gulducat
Copy link
Member

gulducat commented Jun 6, 2023

By my eye, that PR ensures that path is present in host_volume config and not an empty string, which is a nice improvement, but it doesn't check for the presence of the directory on disk.

Seems the latter is the desired outcome of the issue, so I'll go ahead and reopen.

Can repro with this (partial) config:

client {
  enabled = true

  host_volume "nope" {
    path = "/nope" # assuming your system lacks this dir
  }
}
$ nomad config validate client.hcl
Configuration is valid!

but when trying to start the agent:

$ nomad agent -config client.hcl
.....
[ERROR] agent: error starting agent: error="client setup failed: node setup failed: failed to validate volume nope, err: stat /nope: no such file or directory"

@gulducat gulducat reopened this Jun 6, 2023
@lgfa29
Copy link
Contributor

lgfa29 commented Jun 7, 2023

Ops, my bad I missed that in the review.

@dttung2905 would you be interested in extending the work you did in #17393 to also check if the host volume path is actually a directory that exists?

@dttung2905
Copy link
Contributor

Sure @lgfa29 , please allow me some time to take a look at it again 🙏

@maxadamo
Copy link
Author

maxadamo commented Jun 7, 2023

@dttung2905 would you be interested in extending the work you did in #17393 to also check if the host volume path is actually a directory that exists?

@lgfa29 , and @dttung2905 for clarity: it's doesn't have to be a directory. It can also be a file.

@lgfa29
Copy link
Contributor

lgfa29 commented Jun 7, 2023

Ah interesting, like a symlink to a directory? I don't think I've ever tried that 😄

@maxadamo
Copy link
Author

maxadamo commented Jun 8, 2023

@lgfa29 I just checked my configuration, to ensure that I was remembering correctly.
Maybe this is different with CSI volumes, but host_volume can also be a file, not only a directory. See below, my configuration:

    "host_volume": [
      {
        "cams_bootstrap_ldif_master": {
          "path": "/depot/cams/ldif.master",
          "read_only": true
        },
        "cams_bootstrap_ldif_replica": {
          "path": "/depot/cams/ldif.replica",
          "read_only": true
        },
        "cams_etc_ldap_master": {
          "path": "/depot/cams/etc_ldap.master"
        },
        "cams_slapd_conf_replica": {
          "path": "/depot/cams/etc_ldap.replica/slapd.conf"
        },
        "cams_var_lib_ldap_master": {
          "path": "/depot/cams/var_lib_ldap.master"
        },
        "cams_local": {
          "path": "/depot/cams/local"
        },
        "cams_comanage_conf": {
          "path": "/depot/cams/apache2/000-comanage.conf",
          "read_only": true
        },
        ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/config type/enhancement
Projects
Status: Needs Roadmapping
Development

Successfully merging a pull request may close this issue.

6 participants