Added a plugin logging configuration to specify defaults.

[apollo13]

This is similar to https://github.com/hashicorp/nomad/pull/10156/files and allows for a configuration like:

plugin "nomad-driver-podman" {
    config {
    extra_labels = ["job_name", "job_id", "task_group_name", "task_name", "namespace", "node_name", "node_id"]
    logging {
        driver = "journald"
        options = {
            tag = "{{index .Config.Labels \"com.hashicorp.nomad.alloc_id\" }}"
        }
    }
    }
}

[apollo13]

Hi @lgfa29, thank you for your review. Can you please explain to me why changing the type affects backwards compat (trying to learn more about how nomad works)?

[lgfa29]

Can you please explain to me why changing the type affects backwards compat (trying to learn more about how nomad works)?

Sure! I should preface this by saying that I'm mostly going from the PR description on https://github.com/hashicorp/nomad/pull/5321 which is where MapStrStr was introduced.

HCL supports two syntax: native HCL and JSON. HCLv1 is somewhat under-specified in some senses, which create a few ambiguities, meaning two different HCL/JSON files may be decoded into the same struct.

https://github.com/hashicorp/nomad/pull/5321 lists some examples:

// hcl block
port_map {
  http = 80
  https = 443
}

// hcl assignment
port_map = {
  http  = 80
  https = 443
}

// json single element array of map (default in API response)
{"port_map": [{"http": 80, "https": 443}]}

// json array of individual maps (supported accidentally iiuc)
{"port_map: [{"http": 80}, {"https": 443}]}

HCLv2 is more strict, and makes a block (port { ... }) and a map assignment (port = { ... }) to be two different things. So, in order to support job files written with the old syntax, MapStrStr was created to manually decoded them the same way.

In this scenario, this means that a Podman driver written like this (note the missing = after options) would no longer work:

task "..." {
  driver = "podman"

  config {
    logging = {
      driver = "journald"
      options {
        "tag" = "redis"
      }
    }
  }
  # ...
}

But it's probably worth testing it by running a task like the above using the latest driver version available and then comparing it to this PR.

It would also be interesting to test an upgrade path.

1. Start non-dev Nomad cluster (need to retain state).
2. Run a Podman job with a task like the above.
3. Stop Nomad client.
4. Update Podman driver with this PR.
5. Start Nomad client and verify if the alloc was able to be recovered.

[apollo13]

Thank you for the explanation @lgfa29. Would be great if you could take the changes for a testrun and maybe merge it :)

EDIT:// lemme fix the tests.

[apollo13]

@lgfa29 So when the plugin config is constructed in tests like this: pluginConfig := PluginConfig{} the defaults from the spec obviously don't apply. I am wondering at which point I should handle an empty driver, would it be here https://github.com/hashicorp/nomad-driver-podman/pull/285/files#diff-d16a4232641fc7ed88a3737c22b4f54d7e03dd7fdf9053530fde70e5bca63fb1R514 or should the tests create a "proper" config. Not sure enough about nomad internals to know how to proceed.

[apollo13]

@lgfa29 👋 would love a review :)

[lgfa29]

Testing went well. I had to rebase the branch to fix a merge conflict in the README, but I think we're good to go!

[apollo13]

Thank you very much and sorry for forgetting to rebase.

[lgfa29]

Ah no worries. I caused the conflict when I pushed a CHANGELOG entry 😅