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.

Sign up for GitHub

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

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
cmd/ore/brightbox/gc.go Outdated Show resolved Hide resolved
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
2 checks passed
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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