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

✨ Support calling provider plugins. #97

Merged
merged 1 commit into from
Mar 15, 2024
Merged

Conversation

guettli
Copy link
Contributor

@guettli guettli commented Mar 14, 2024

What this PR does / why we need it:

We need a way to call csmctl plugins which handle the provider specific parts (like creating node-images).

Which issue(s) this PR fixes

Fixes #4

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Testing

You can see how the provider plugin docker gets called with this command:

make build; ./csctl create tests/cluster-stacks/docker/ferrol -m hash   

TODOs:

  • squash commits
  • include documentation
  • add unit tests

@cluster-stack-bot cluster-stack-bot bot added size/M Denotes a PR that changes 50-200 lines, ignoring generated files. area/code Changes made in the code directory area/hack Changes made in the hack directory labels Mar 14, 2024
@guettli guettli mentioned this pull request Mar 14, 2024
3 tasks
@guettli guettli force-pushed the tg/call-csmplugin-redone branch 2 times, most recently from dae4219 to 1ab3abc Compare March 14, 2024 14:03
Signed-off-by: Thomas Guettler <thomas.guettler@syself.com>
@janiskemper
Copy link
Member

looks good to me. @michal-gubricky you can test this with openstack!
@guettli if you have some spare time, you could see that you include a "--node-image-registry" flag and also template it along ClusterStackVersion, etc..

We would like to use "<< .NodeImageRegistry >>" as key in our templates!
If @michal-gubricky is very quick, we can also do that in another PR

@michal-gubricky
Copy link
Contributor

michal-gubricky commented Mar 14, 2024

looks good to me. @michal-gubricky you can test this with openstack! @guettli if you have some spare time, you could see that you include a "--node-image-registry" flag and also template it along ClusterStackVersion, etc..

I tested it with openstack plugin, but something strange appeared. I do not know if something is wrong in my setup or if there is a bug in csctl. When I have this folder structure node-images/control-plane-ubuntu-2204 there are packer-specific scripts and templates the csctl returns error(see details). Besides this, it seems to be working when I do not have this folder there. The problematic file is taken from image-builder repo, see user-data.

Details

Error: failed to generate release: failed to generate tmp output: failed to walk files: failed to create new template: Cannot find end tag=" >>" in the template="#cloud-config\n# Copyright 2022 The Kubernetes Authors.\n#\n# Licensed under the Apache License, Version 2.0 (the \"License\");\n# you may not use this file except in compliance with the License.\n# You may obtain a copy of the License at\n#\n# http://www.apache.org/licenses/LICENSE-2.0\n#\n# Unless required by applicable law or agreed to in writing, software\n# distributed under the License is distributed on an \"AS IS\" BASIS,\n# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n# See the License for the specific language governing permissions and\n# limitations under the License.\n\n\n# For more information on how autoinstall is configured, please refer to\n# https://ubuntu.com/server/docs/install/autoinstall-reference\nautoinstall:\n version: 1\n # Disable ssh server during installation, otherwise packer tries to connect and exceed max attempts\n early-commands:\n - systemctl stop ssh\n # Configure the locale\n locale: en_US.UTF-8\n keyboard:\n layout: us\n # Create a single-partition with no swap space. Kubernetes\n # really dislikes the idea of anyone else managing memory.\n # For more information on how partitioning is configured,\n # please refer to https://curtin.readthedocs.io/en/latest/topics/storage.html.\n storage:\n grub:\n replace_linux_default: false\n config:\n - type: disk\n id: disk-0\n size: largest\n grub_device: true\n preserve: false\n ptable: msdos\n wipe: superblock\n - type: partition\n id: partition-0\n device: disk-0\n size: -1\n number: 1\n preserve: false\n flag: boot\n - type: format\n id: format-0\n volume: partition-0\n fstype: ext4\n preserve: false\n - type: mount\n id: mount-0\n device: format-0\n path: /\n updates: 'all'\n ssh:\n install-server: true\n allow-pw: true\n # Create the default user.\n # Ensures the \"builder\" user doesn't require a password to use sudo.\n user-data:\n users:\n - name: builder\n # openssl passwd -6 -stdin <<< builder\n passwd: $6$xyz$UtXVazU08Q5b8AW.TJ3MPYZglyXa3Ttf2RCel8MCUPlEYO1evWxeWBhZ2QqivU/Ij4tqYAxMCqc2ujEM4dMSe1\n groups: [adm, cdrom, dip, plugdev, lxd, sudo]\n lock-passwd: false\n sudo: ALL=(ALL) NOPASSWD:ALL\n shell: /bin/bash\n\n # This command runs after all other steps; it:\n # 1. Disables swapfiles\n # 2. Removes the existing swapfile\n # 3. Removes the swapfile entry from /etc/fstab\n # 4. Cleans up any packages that are no longer required\n # 5. Removes the cached list of packages\n late-commands:\n - swapoff -a\n - rm -f /swapfile\n - sed -ri '/\\sswap\\s/s/^#?/#/' /etc/fstab\n - apt-get purge --auto-remove -y\n - rm -rf /var/lib/apt/lists/*" starting from "builder\n passwd: $6$xyz$UtXVazU08Q5b8AW.TJ3MPYZglyXa3Ttf2RCel8MCUPlEYO1evWxeWBhZ2QqivU/Ij4tqYAxMCqc2ujEM4dMSe1\n groups: [adm, cdrom, dip, plugdev, lxd, sudo]\n lock-passwd: false\n sudo: ALL=(ALL) NOPASSWD:ALL\n shell: /bin/bash\n\n # This command runs after all other steps; it:\n # 1. Disables swapfiles\n # 2. Removes the existing swapfile\n # 3. Removes the swapfile entry from /etc/fstab\n # 4. Cleans up any packages that are no longer required\n # 5. Removes the cached list of packages\n late-commands:\n - swapoff -a\n - rm -f /swapfile\n - sed -ri '/\\sswap\\s/s/^#?/#/' /etc/fstab\n - apt-get purge --auto-remove -y\n - rm -rf /var/lib/apt/lists/*"

@guettli
Copy link
Contributor Author

guettli commented Mar 14, 2024

looks good to me. @michal-gubricky you can test this with openstack! @guettli if you have some spare time, you could ..

@janiskemper yes, that was my plan, too. But I would do that in a second PR. Can you please review the current state.

@guettli
Copy link
Contributor Author

guettli commented Mar 14, 2024

@michal-gubricky I will take a look at that tomorrow. Maybe << or >> is in your cloud init config file (for example in bash heredoc). But csctl should be able to handle this.

@michal-gubricky
Copy link
Contributor

@michal-gubricky I will take a look at that tomorrow. Maybe << or >> is in your cloud init config file (for example in bash heredoc). But csctl should be able to handle this.

@guettli After removing <<< on line 68 csctl doesn't throw this error. But as you mentioned, I also think that csctl should be able to handle this, especially since character <<< was only part of a comment.

@guettli
Copy link
Contributor Author

guettli commented Mar 15, 2024

@michal-gubricky Thank you for feedback. I created an issue, because csctl must not fail if >> or << is somewhere in the template: #98

@guettli guettli merged commit 6b4e70e into main Mar 15, 2024
5 checks passed
@guettli guettli deleted the tg/call-csmplugin-redone branch March 15, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code Changes made in the code directory area/hack Changes made in the hack directory size/M Denotes a PR that changes 50-200 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement plugin mechanism for providers
3 participants