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

schema: add JSON schema for mcollective, migrator and mounts modules #1358

Merged
merged 6 commits into from
Mar 30, 2022

Conversation

blackboxsw
Copy link
Collaborator

@blackboxsw blackboxsw commented Mar 29, 2022

Proposed Commit Message

schema: add JSON schema for mcollective, migrator and mounts modules

Define static JSON schema for cloud-config modules in
cloudinit/config/cloud-init-schema.json. Migrate existing doc/examples
into inline meta schema examples where applicable.

Includes:
 - cc_mcollective
 - cc_migrator
 - cc_mounts

Doc drops:
 - doc/examples/cloud-config-mcollective.txt because example is included in
   inline meta schema docs

Additional Context

Test Steps

tox -re doc; xdg-open doc/rtd_html/topics/modules.html 
# validate visual changes vs https://cloudinit.readthedocs.io/en/latest/topics/modules.html#mcollective

# validate specific module rendered docs on cmdline
PYTHONPATH=. python3 -m cloudinit.cmd.main devel schema --docs cc_mcollective

Unit tests will run through any included cc_.meta['examples'] to confirm that they are correct schema.

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

@blackboxsw blackboxsw requested a review from holmanb March 29, 2022 19:27
@blackboxsw blackboxsw force-pushed the schema-m branch 2 times, most recently from d95b523 to 2aa230f Compare March 29, 2022 22:40
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Some comments inline, but overall looking good.

cloudinit/config/cloud-init-schema.json Show resolved Hide resolved
cloudinit/config/cloud-init-schema.json Outdated Show resolved Hide resolved
cloudinit/config/cc_mounts.py Show resolved Hide resolved
cloudinit/config/cc_mounts.py Show resolved Hide resolved
cloudinit/config/cloud-init-schema.json Outdated Show resolved Hide resolved
},
"size": {
"description": "The size in bytes of the swap file, or 'auto'",
"oneOf": [{"enum": ["auto"]}, {"type": "integer"}]
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

whoa nice find will add a positive example here in meta schema and fix this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added "pattern": "^([0-9]+)?\\.?[0-9]+[BKMGT]$"} to schema for swap.size swap.maxsize and unittests

cloudinit/config/cloud-init-schema.json Outdated Show resolved Hide resolved
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

One typo we should fix, and everything else optional. For some reason Github wouldn't let me do suggestions on this review.

cloudinit/config/cloud-init-schema.json Outdated Show resolved Hide resolved
cloudinit/config/cc_mounts.py Outdated Show resolved Hide resolved
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Looks good!

@TheRealFalcon TheRealFalcon merged commit 24977e9 into canonical:main Mar 30, 2022
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