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

feat: add initial sources #1

Merged
merged 1 commit into from
Jul 28, 2024

Conversation

utkuozdemir
Copy link
Member

@utkuozdemir utkuozdemir commented Jul 13, 2024

The initial implementation.

Depends on: siderolabs/omni#449

@utkuozdemir utkuozdemir force-pushed the initial-implementation branch 2 times, most recently from 94acf85 to 6d0cca7 Compare July 13, 2024 08:34
.kres.yaml Outdated
@@ -2,6 +2,7 @@
kind: common.Image
name: image-omni-cloud-provider-qemu
spec:
baseImage: ghcr.io/siderolabs/talosctl:v1.7.5
Copy link
Member Author

Choose a reason for hiding this comment

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

This provisioner needs talosctl, as the provisioning code imported from Talos requires it to spawn more processes over it. The talosctl binary is searched in the working directory and in the PATH by this provisioner. When we use it as the base image, it is found in the current working directory.

Copy link
Member

Choose a reason for hiding this comment

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

i think we can just use a copy stage and copy over from the talosctl image

Copy link
Member Author

Choose a reason for hiding this comment

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

I look into that, but it looked like I can copy over from other images (in the same dockerfile), but not from an arbitrary image. Might be wrong though.

Also, does this have any downsides? There is nothing in the talosctl base image other than talosctl.

Comment on lines +7 to +15
message QemuMachineSpec {
string uuid = 1;
string disk_path = 2;
}

message QemuMachineAllocationSpec {
string talos_version = 1;
string schematic_id = 2;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the resources internal to this provider, living in the cloud-provider:qemu namespace of Omni's state.

rootCmd.Flags().StringSliceVar(&rootCmdArgs.nameservers, "nameservers", []string{"1.1.1.1", "1.0.0.1"}, "the nameservers to use for the QEMU VMs.")
rootCmd.Flags().StringVar(&rootCmdArgs.imageFactoryPXEURL, "image-factory-pxe-url", "https://pxe.factory.talos.dev", "the URL of the image factory PXE server.")
rootCmd.Flags().IntVar(&rootCmdArgs.ipxeServerPort, "ipxe-server-port", 42420, "the port of the iPXE server.")
rootCmd.Flags().IntVar(&rootCmdArgs.numMachines, "num-machines", 8, "the number of machines to provision.")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the total pool of machines the provider provisions on startup. They go into an ipxe boot loop until they are assigned to a MachineRequest.

@utkuozdemir utkuozdemir force-pushed the initial-implementation branch 6 times, most recently from ec60dda to 9537f35 Compare July 13, 2024 08:56
hack/release.toml Outdated Show resolved Hide resolved
@utkuozdemir utkuozdemir force-pushed the initial-implementation branch 5 times, most recently from 43f29aa to b937774 Compare July 17, 2024 08:19
func (provider *Provider) watchSysVersion(ctx context.Context, st state.State) error {
eventCh := make(chan state.Event)

if err := st.Watch(ctx, system.NewSysVersion(omniresources.EphemeralNamespace, system.SysVersionID).Metadata(), eventCh); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should add a proper health endpoint.

Copy link
Member Author

@utkuozdemir utkuozdemir Jul 21, 2024

Choose a reason for hiding this comment

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

Yep, that's a good idea. Here the problem I work around is, when Omni gets restarted, the state I have over the client API gets broken, but the runtime doesn't become aware of it. I couldn't find a way to make it aware of it (using GRPC options etc.). Maybe there is one, but I couldn't find any.

So not sure if it's an Omni problem - maybe a COSI one?

@utkuozdemir
Copy link
Member Author

Holding this until the Omni side gets merged

The initial implementation.

Signed-off-by: Utku Ozdemir <utku.ozdemir@siderolabs.com>
@utkuozdemir
Copy link
Member Author

/m

@talos-bot talos-bot merged commit 335731d into siderolabs:main Jul 28, 2024
13 checks passed
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.

4 participants