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

Add support for a nat-less libvirt deployment and multiple workers #65

Merged
merged 2 commits into from
Mar 19, 2021

Conversation

detiber
Copy link
Contributor

@detiber detiber commented Mar 17, 2021

Description

Allows for deploying the vagrant/libvirt setup without NAT and with multiple workers, which enables testing with cluster-api-provider-tink

Why is this needed

Helps with testing CAPT

How Has This Been Tested?

Currently testing at the moment, but all testing will consist of manual testing with vagrant/libvirt

How are existing users impacted? What migration steps/scripts do we need?

This could affect existing vagrant/libvirt users if they have an existing worker running when they update, not sure if there is a good way to avoid that, though.

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@gauravgahlot gauravgahlot added this to In Progress in Issues List via automation Mar 18, 2021
@gianarb gianarb added the ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ label Mar 18, 2021
Copy link
Contributor

@gianarb gianarb left a comment

Choose a reason for hiding this comment

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

Hello! Great work this change breaks our CI/CD because it changes the name of the worker from worker to worker-1. Can you update the test suite as well? I am not sure if somewhere in the documentation is mentioned worker instead of worker-1, if so we have to update documentation as well

@detiber
Copy link
Contributor Author

detiber commented Mar 18, 2021

Hello! Great work this change breaks our CI/CD because it changes the name of the worker from worker to worker-1. Can you update the test suite as well? I am not sure if somewhere in the documentation is mentioned worker instead of worker-1, if so we have to update documentation as well

Updated to avoid the renaming of the first worker, the first worker will remain named worker and subsequent workers are named worker<n>

@gianarb gianarb added ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ and removed ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ labels Mar 18, 2021
@gianarb gianarb added the ready-to-merge Signal to Mergify to merge the PR. label Mar 19, 2021
@mergify mergify bot merged commit 193a2fa into tinkerbell:master Mar 19, 2021
Issues List automation moved this from In Progress to Just shipped Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ ready-to-merge Signal to Mergify to merge the PR.
Projects
No open projects
Issues List
  
Just shipped
Development

Successfully merging this pull request may close these issues.

None yet

3 participants