Skip to content

Fix ds-identify not detecting NoCloud seed in config (SC-542)#1381

Merged
TheRealFalcon merged 1 commit into
canonical:mainfrom
TheRealFalcon:nocloud
Apr 25, 2022
Merged

Fix ds-identify not detecting NoCloud seed in config (SC-542)#1381
TheRealFalcon merged 1 commit into
canonical:mainfrom
TheRealFalcon:nocloud

Conversation

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Proposed Commit Message

Fix ds-identify not detecting NoCloud seed in config

NoCloud seed config can be defined in /etc/cloud/cloud.cfg[.d].
However, ds-identify had no means of detecting this config and reported
NOT FOUND. This commit allows ds-identify to detect and report it
properly.

LP: #1876375

Additional Context

LP: #1876375 notes that if you specify the datasource list in a cfg file, it works, but if you don't it doesn't. It has nothing to do with the reading of the config files. If the datasource list only contains one entry (or one + None), ds-identify skips datasource detection entirely and just uses the specified datasource. https://github.com/canonical/cloud-init/blob/main/tools/ds-identify#L1793-L1798 . The problem is that ds-identify just had no means of detecting this use case.

Test Steps

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

NoCloud seed config can be defined in /etc/cloud/cloud.cfg[.d].
However, ds-identify had no means of detecting this config and reported
NOT FOUND. This commit allows ds-identify to detect  and report it
properly.

LP: #1876375
Comment thread tools/ds-identify
fi

# This is a bit hacky, but a NoCloud false positive isn't the end of the world
if check_config "NoCloud" && check_config "user-data" && check_config "meta-data"; then
Copy link
Copy Markdown
Member

@holmanb holmanb Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user-data and meta-data are required seed files for NoCloud, but it looks like check_config searches /etc/cloud/cloud.cfg and /etc/cloud/cloud.cfg.d/* for keys with this name (am I reading that right?). Since I don't see those as keys in the doc, could you please expand on why this works?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this functionality needs to be documented better. I just learned of this behavior due to the bug report 😄 The only documentation is an example. The magic happens in the datasource.

For the ds-identify piece, check_config just looks for a key at any nesting of the config (thanks bash). I'm basically just assuming that if 'NoCloud', 'user-data', and 'meta-data' exist somewhere in the config, then the datasource definition is happening there. If we're wrong, we waste a few cycles in python code checking for NoCloud.

Copy link
Copy Markdown
Contributor Author

@TheRealFalcon TheRealFalcon Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g., create
/etc/cloud/cloud.cfg.d/99_ds.cfg:

datasource_list: ["NoCloud"]
datasource:
  NoCloud:
    user-data:  |
      #cloud-config
      runcmd: 
      - echo 'hi' > /var/tmp/hi 
    meta-data:
      instance-id: i-87018aed
      local-hostname: myhost.internal

Then cloud-init clean --logs --reboot and you should see those details reflected in your instance. With this branch, you shouldn't need the datasource_list line.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! Thanks. Yes this seems like something that should be documented better. I'm indifferent whether we should update the docs in this PR or a followup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already in our docs tracking ticket

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