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

chore: import siderolink as siderolink-launch subcommand. #8482

Merged
merged 1 commit into from
Mar 23, 2024

Conversation

DmitriyMV
Copy link
Member

This PR ensures that we can test our siderolink communication using embedded siderolink-agent. If --with-siderolink provided during talos cluster create talosctl will embed proper kernel string and setup siderolink-agent as a separate process. It should be used with combination of --skip-injecting-config and --with-apply-config (the latter will use newly generated IPv6 siderolink addresses which talosctl passes to the agent as a "pre-bind").

@DmitriyMV DmitriyMV force-pushed the siderolink-agent branch 3 times, most recently from 60f2003 to c638d32 Compare March 21, 2024 18:48
apiLink := "grpc://" + net.JoinHostPort(wgHost, apiPort) + "?jointoken=foo"

extraKernelArgs.Append("siderolink.api", apiLink)
extraKernelArgs.Append("talos.events.sink", "[fdae:41e4:649b:9303::1]:"+sinkPort)
Copy link
Member

Choose a reason for hiding this comment

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

probably makes sense to define this IP as a constant

Copy link
Member

Choose a reason for hiding this comment

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

in fact, we should calculate it or pull from the library

Copy link
Member Author

Choose a reason for hiding this comment

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

Done - we now use GetAgentNodeAddr() which method which uses generated prefix from wireguard.NetworkPrefix("").

pkg/provision/options.go Outdated Show resolved Hide resolved
@smira
Copy link
Member

smira commented Mar 22, 2024

we should add an actual test (e.g. to integration-misc) which boots with SideroLink and applies the config, otherwise we're not doing the thing fully.

@DmitriyMV DmitriyMV force-pushed the siderolink-agent branch 2 times, most recently from a0e2188 to be2d33a Compare March 22, 2024 19:14
.drone.jsonnet Outdated
@@ -644,6 +644,13 @@ local integration_cloud_images = Step('cloud-images', depends_on=[integration_im

local integration_reproducibility_test = Step('reproducibility-test', target='reproducibility-test', depends_on=[load_artifacts], environment={ IMAGE_REGISTRY: local_registry });

local integration_siderolink = Step('e2e-siderolink', target='e2e-qemu', privileged=true, depends_on=[load_artifacts], environment={
Copy link
Member

Choose a reason for hiding this comment

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

it should depends_on on the previous set (default-hostname in this case)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

apiLink := "grpc://" + net.JoinHostPort(wgHost, apiPort) + "?jointoken=foo"

extraKernelArgs.Append("siderolink.api", apiLink)
extraKernelArgs.Append("talos.events.sink", "[fdae:41e4:649b:9303::1]:"+sinkPort)
Copy link
Member

Choose a reason for hiding this comment

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

let's use proper net.JoinHostPort instead of this, and the IP we should know from the SideroLink library

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. We now use GetAgentNodeAddr which method which uses generated prefix fromwireguard.NetworkPrefix("").

return errors.New("siderolink kernel arguments are already set, cannot run with --with-siderolink")
}

wgPort := rand.IntN(20000) + 40000
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. We now generate ports in getDynamicPorts which uses getDynamicPort and checks that generated ports do not overlap.

@@ -1153,6 +1208,8 @@ func init() {
createCmd.Flags().IntVar(&bandwidth, "with-network-bandwidth", 0, "specify bandwidth restriction (in kbps) on the bridge interface when creating a qemu cluster")
createCmd.Flags().StringVar(&withFirewall, firewallFlag, "", "inject firewall rules into the cluster, value is default policy - accept/block (QEMU only)")
createCmd.Flags().BoolVar(&withUUIDHostnames, "with-uuid-hostnames", false, "use machine UUIDs as default hostnames (QEMU only)")
createCmd.Flags().BoolVar(&withSiderolinkAgent, "with-siderolink", false, "enables the use of siderolink agent as configuration apply mechanism")
createCmd.Flags().MarkHidden("with-siderolink") //nolint:errcheck
Copy link
Member

Choose a reason for hiding this comment

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

why hidden? we should have it

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

}

var siderolinkCmd = &cobra.Command{
Use: "siderolink-launch",
Copy link
Member

Choose a reason for hiding this comment

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

why do we need another folder? can we put it next to other *-launch commands, and we can make the whole file //go:build linux to have less moving parts

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Moved to cmd/talosctl/cmd/mgmt/siderolink_launch_linux.go

@DmitriyMV DmitriyMV force-pushed the siderolink-agent branch 4 times, most recently from bc2f70a to 4b675ed Compare March 23, 2024 11:38
@DmitriyMV
Copy link
Member Author

/promote integration-misc

This PR ensures that we can test our siderolink communication using embedded siderolink-agent.
If `--with-siderolink` provided during `talos cluster create` talosctl will embed proper kernel string and setup `siderolink-agent` as a separate process. It should be used with combination of `--skip-injecting-config` and `--with-apply-config` (the latter will use newly generated IPv6 siderolink addresses which talosctl passes to the agent as a "pre-bind").

Fixes siderolabs#8392

Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
Copy link
Member

@smira smira left a comment

Choose a reason for hiding this comment

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

🆒

@DmitriyMV
Copy link
Member Author

/m

@talos-bot talos-bot merged commit 949ad11 into siderolabs:main Mar 23, 2024
18 checks passed
@DmitriyMV DmitriyMV deleted the siderolink-agent branch March 23, 2024 13:49
@DmitriyMV DmitriyMV linked an issue Apr 1, 2024 that may be closed by this pull request
@DmitriyMV DmitriyMV self-assigned this Apr 1, 2024
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.

integrate siderolink-agent into talosctl cluster create
4 participants