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 metadata and ipamConfig to worker configuration #39

Merged
merged 3 commits into from
Jan 28, 2025
Merged

Conversation

defo89
Copy link
Contributor

@defo89 defo89 commented Dec 26, 2024

Proposed Changes

Fixes #

@defo89 defo89 added the enhancement New feature or request label Dec 26, 2024
@defo89 defo89 requested a review from a team as a code owner December 26, 2024 16:09
@defo89 defo89 marked this pull request as draft December 26, 2024 16:09
@github-actions github-actions bot added documentation Improvements or additions to documentation provider labels Dec 26, 2024
@defo89 defo89 changed the title worker metadata and ipam PoC: worker metadata and ipam Dec 26, 2024
for _, networkRef := range providerSpec.AddressesFromNetworks {
// check if IPAddress exists
ipAddr := &ipamv1alpha1.IP{}
ipAddrName := req.Machine.Name
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is a naming collision, if a machine requests more than one IP.

Comment on lines +89 to +95
if err = d.metalClient.Get(ctx, ipAddrKey, ipAddr); err != nil && !apierrors.IsNotFound(err) {
return nil, err
}
if err == nil {
klog.V(3).Infof("IP found %s", ipAddrName)
}
if apierrors.IsNotFound(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks quite complicated to me. What about the following?

err = d.metalClient.Get()
if err == nil {
    // append ip
    continue
}
if !apierrors.IsNotFound(err) {
    return nil, err
}
// IP creation path + append ip

@@ -45,6 +47,18 @@ func (d *metalDriver) DeleteMachine(ctx context.Context, req *driver.DeleteMachi
return nil, status.Error(codes.Unknown, fmt.Sprintf("error deleting ignition secret: %s", err.Error()))
}

ip := &ipamv1alpha1.IP{
ObjectMeta: metav1.ObjectMeta{
Name: req.Machine.Name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, needs a change to support multiple IP objects per node.

@@ -32,4 +34,15 @@ type ProviderSpec struct {
DnsServers []netip.Addr `json:"dnsServers,omitempty"`
// ServerLabels are passed to the ServerClaim to find a server with certain properties
ServerLabels map[string]string `json:"serverLabels,omitempty"`
// MedaData is a key-value map of additional data which should be passed to the Machine.
Copy link
Member

Choose a reason for hiding this comment

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

Typo

@github-actions github-actions bot removed the size/L label Jan 28, 2025
@Nuckal777 Nuckal777 marked this pull request as ready for review January 28, 2025 10:08
@afritzler afritzler changed the title PoC: worker metadata and ipam Add worker metadata and ipamConfig Jan 28, 2025
@afritzler afritzler changed the title Add worker metadata and ipamConfig Add metadata and ipamConfig to worker configuration Jan 28, 2025
@Nuckal777 Nuckal777 merged commit b853fd7 into main Jan 28, 2025
10 checks passed
@Nuckal777 Nuckal777 deleted the enh-worker-ipam branch January 28, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request provider
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants