Weird cli interface of nydus-image unpack (and also compact)

[SToPire]

The following code snippet, when determining the location of the blob file, will first look for the blob file based on the locations specified by --blob-dir and --blob, and finally attempt to use --backend-config.

https://github.com/dragonflyoss/nydus/blob/36844742543f69ec5ef32e1f416d38a4fed680db/src/bin/nydus-image/main.rs#L1479-L1514

However, the parameters accepted by --backend-config are still the configuration files for nydusd, from which the backend configuration is then obtained. This logic seems strange and redundant, because the --config parameter is also used to read the nydusd configuration file.

https://github.com/dragonflyoss/nydus/blob/36844742543f69ec5ef32e1f416d38a4fed680db/src/bin/nydus-image/main.rs#L1778-L1790

According to the following code snippet it seems that --backend-config is deprecated and only maintained for backward compatibility.

https://github.com/dragonflyoss/nydus/blob/36844742543f69ec5ef32e1f416d38a4fed680db/src/bin/nydus-image/main.rs#L1702-L1711

I doubt whether it is necessary for us to introduce the --backend-config option in 081c8f968bcb9304c7a4caaa5ce038e54434cd69, especially considering that nydus-image only interacts with local files. Could we just remove it to keep it consistent with the other subcommand in nydus-image?

[imeoer]

Thanks for the discovery, I think the unpack and compact subcommands is unnecessary for getting the complete nydusd configuration file, as they only require reading chunks from the backend, this can be done whether the backend is localfs (by --blob or --blob-dir) or other backends such as oss or registry. It seems a bug in the get_backend func introduced from historical PRs.

[SToPire]

Thanks for your reply. @imeoer Would you mind I submit a PR to remove the --backend-config option, or keep it by modifying get_backend, making it handle the backend configuration file directly rather than nydusd configuration file?

If you think the latter is better, please let me know what does a 'backend configuration file' looks like?

May it look like the ["device"]["backend"] field in nydusd configuration json file? (i.e., "type" + "config")?

{
  "device": {
    "backend": {
      "type": "localfs",
      "config": {
        "dir": "/var/lib/nydus/blobs"
      }
    },
    "cache": {
      "type": "blobcache",
      "config": {
        "work_dir": "/var/lib/nydus/cache"
      }
    }
  },
  "mode": "direct",
  "digest_validate": false,
  "iostats_files": false,
  "enable_xattr": true
}

Or looks like the example mentioned in nydus/docs/nydus-image.md? (I am afraid this section of document is out of date now.)

{
  "endpoint": "region.aliyuncs.com",
  "scheme": "https",
  "access_key_id": "",
  "access_key_secret": "",
  "bucket_name": "",
  "object_prefix": "nydus/"
}

[imeoer]

Thanks for your reply. @imeoer Would you mind I submit a PR to remove the --backend-config option, or keep it by modifying get_backend, making it handle the backend configuration file directly rather than nydusd configuration file?

How about defining --backend-config, --backend-config-file and --backend-type options, and removing --config? Keep it same with nydusify convert/check/..., two usage mode:

• --backend-type oss --backend-config '{ ... }' // JSON encoded backend config string.
• --backend-type oss --backend-config-file /path/to/oss-backend.json // JSON encoded backend config file.

For example:

--backend-type oss --backend-config-file /path/to/oss-backend.json, the /path/to/oss-backend.json looks like:

{
  "endpoint": "region.aliyuncs.com",
  "scheme": "https",
  "access_key_id": "",
  "access_key_secret": "",
  "bucket_name": "",
  "object_prefix": "nydus/"
}

--backend-type localfs --backend-config-file /path/to/localfs-backend.json, the /path/to/localfs-backend.json looks like:

{
  "dir": "/var/lib/nydus/blobs"
}

or using a quick option for localfs backend: --blob /path/to/single-blob / --blob-dir /path/to/blob-dir.