Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

kola: add brightbox support #484

Merged
merged 9 commits into from
Nov 23, 2023
Merged

kola: add brightbox support #484

merged 9 commits into from
Nov 23, 2023

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Nov 15, 2023 •
edited
Loading

This PR adds Brightbox support to Mantle, in order to test Flatcar on this provider (nightly or before a release, let's discuss this later).

How to use

We first need to create the image on Brightbox.

Testing done

$ BRIGHTBOX_IMAGE_ID=$(ore brightbox --brightbox-client-id ... --brightbox-client-secret ... create-image --name flatcar-nigthly --url http://bincache.flatcar-linux.net/images/amd64/3791.0.0+nightly-20231117-2100/flatcar_production_openstack_image.img)
$ time kola --parallel 1 --platform brightbox --brightbox-client-id ... --brightbox-client-secret ... --brightbox-image ${BRIGHTBOX_IMAGE_ID} run '*'
PASS, output in _kola_temp/brightbox-2023-11-20-1508-31363

real	42m42.941s
user	0m0.039s
sys	0m0.024s
$ cat _kola_temp/brightbox-latest/test.tap
1..21
ok - docker.network
ok - cl.etcd-member.discovery
ok - coreos.ignition.resource.remote
ok - cl.internet
ok - cl.install.cloudinit
ok - cl.flannel.udp
ok - coreos.ignition.sethostname
ok - cl.network.initramfs.second-boot
ok - kubeadm.v1.26.5.flannel.cgroupv1.base
ok - cl.ignition.kargs
ok - coreos.ignition.security.tls
ok - cl.ignition.v2.btrfsroot
ok - coreos.ignition.once
ok - kubeadm.v1.28.1.flannel.base
ok - cl.flannel.vxlan
ok - cl.basic
ok - coreos.ignition.ssh.key
ok - coreos.ignition.resource.local
ok - kubeadm.v1.26.5.flannel.base
ok - cl.cloudinit.basic
ok - kubeadm.v1.27.2.flannel.base

NOTE:

  • Fetching console output is not yet supported / implemented at the moment on the provider side

@tormath1
mod: add brightbox/v2 dependency
2288193 
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
vendor: go mod tidy && go mod vendor
930a7ef 
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1 tormath1 mentioned this pull request Nov 15, 2023
Closed
@pothos
Copy link
Member

pothos commented Nov 15, 2023

For completeness we also need ore support to delete all VMs/images(/IPs?). That's good for a cleanup before kola starts when we don't have unlimited VM slots. It's also good to have a GC job via ore that removes unused resources.

@tormath1 tormath1 force-pushed the tormath1/brightbox branch 4 times, most recently from e53791d to 4e78735 Compare November 17, 2023 13:37
@tormath1 tormath1 self-assigned this Nov 17, 2023
@tormath1 tormath1 marked this pull request as ready for review November 17, 2023 14:42
@tormath1 tormath1 requested a review from a team November 17, 2023 14:42
pothos
pothos reviewed Nov 17, 2023
View reviewed changes
pothos
pothos reviewed Nov 17, 2023
View reviewed changes
platform/machine/brightbox/machine.go Outdated
Comment on lines 85 to 88
if cloudIP != "" {
plog.Infof("Adding Cloud IP to the pool: %s", cloudIP)
bm.cluster.flight.cloudIPs <- cloudIP
}
Copy link
Member

Choose a reason for hiding this comment

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

I would move this down to be the last action of the function.

@tormath1
Copy link
Contributor Author

All tests are passing with latest nightly:

$ BRIGHTBOX_IMAGE_ID=$(ore brightbox --brightbox-client-id ... --brightbox-client-secret ... create-image --name flatcar-nigthly --url http://bincache.flatcar-linux.net/images/amd64/3791.0.0+nightly-20231117-2100/flatcar_production_openstack_image.img)
$ time kola --parallel 1 --platform brightbox --brightbox-client-id ... --brightbox-client-secret ... --brightbox-image ${BRIGHTBOX_IMAGE_ID} run '*'
PASS, output in _kola_temp/brightbox-2023-11-20-1508-31363

real	42m42.941s
user	0m0.039s
sys	0m0.024s
$ cat _kola_temp/brightbox-latest/test.tap
1..21
ok - docker.network
ok - cl.etcd-member.discovery
ok - coreos.ignition.resource.remote
ok - cl.internet
ok - cl.install.cloudinit
ok - cl.flannel.udp
ok - coreos.ignition.sethostname
ok - cl.network.initramfs.second-boot
ok - kubeadm.v1.26.5.flannel.cgroupv1.base
ok - cl.ignition.kargs
ok - coreos.ignition.security.tls
ok - cl.ignition.v2.btrfsroot
ok - coreos.ignition.once
ok - kubeadm.v1.28.1.flannel.base
ok - cl.flannel.vxlan
ok - cl.basic
ok - coreos.ignition.ssh.key
ok - coreos.ignition.resource.local
ok - kubeadm.v1.26.5.flannel.base
ok - cl.cloudinit.basic
ok - kubeadm.v1.27.2.flannel.base

pothos
pothos reviewed Nov 20, 2023
View reviewed changes
platform/api/brightbox/api.go Outdated
for _, cloudIP := range cloudIPs {
// Do not remove a mapped cloud IP - otherwise, we can end up
// with a server without public IP.
if cloudIP.Status == cloudipstatus.Mapped {
Copy link
Member

Choose a reason for hiding this comment

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

I thought that makes sense to ensure that the kola run can use all available IP addrs. I guess we would need a "delete-vms" ore command to also run before this here in the Jenkins job.

Copy link
Member

Choose a reason for hiding this comment

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

Because the garbage collection comes too late when a job restarts after kola crashed.

pothos
pothos approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@pothos pothos left a comment

Choose a reason for hiding this comment

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

Thanks, looks good - I think for robustness we need a way of removing all leftover VMs with ore. We could also use the ore gc subcommand with a duration of 0 as workaround (if we run it before creating our image).

@tormath1
api/brightbox: initial commit
3d48031 
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
ore/brightbox: add new subcommand
e854854 
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
kola: implement brightbox platform
1fd7edc 
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
kubeadm: skip calico / cilium tests for now on brightbox
bced13d 
once we switch to butane, we can enable them.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
ignition/empty: exclude brightbox
51b2fc4 
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
platform/brightbox: remove cloud ips from ore
0b16bb4 
Garbage collection can't handle yet the removal of cloud IPs - we delete
cloud IPs with ore.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
platform/brightbox: add remove-servers ore subcommand
0d1cebd 
this has to run before running a test

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1
Copy link
Contributor Author

tormath1 commented Nov 21, 2023 •
edited
Loading

@pothos thanks for the review. I added a remove-servers subcommand to run before a test. Also seized the opportunity to start this new provider with a good context habit by exposing the context to the new Brightbox API methods to remove a good amount of TODO().

@github-actions github-actions bot mentioned this pull request Nov 22, 2023
Closed
@tormath1 tormath1 merged commit 605ed7c into flatcar-master Nov 23, 2023
@tormath1 tormath1 deleted the tormath1/brightbox branch November 23, 2023 11:14
@github-actions github-actions bot mentioned this pull request Dec 22, 2023
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@pothos pothos pothos approved these changes

Assignees

@tormath1 tormath1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tormath1 @pothos