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

ncm-network: nmstate support for configuring dummy interfaces #1614

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

aka7
Copy link
Contributor

@aka7 aka7 commented Aug 9, 2023

providing support for configuring dummy interfaces using nmstate module.

  • Why the change is necessary.
    internally we use frr to configure vips and routing, ffr configuration are using pan, this change is to allow ncm-network to create the loopback interface for the vips defined. I don't see anything upstream about creating this type of interface, so schema change should not impact anything in network.pm.

  • What backwards incompatibility it may introduce.
    none, change is to nmstate.pm which shouldn't impact current network module.

@jrha jrha added this to the 23.next milestone Aug 10, 2023
ncm-network/src/main/perl/nmstate.pm Outdated Show resolved Hide resolved
ncm-network/src/main/perl/nmstate.pm Outdated Show resolved Hide resolved
support for configuring dummy interfaces using nmstate module.
only if manage_vips is set.
@jrha jrha modified the milestones: 23.next, 23.6 Aug 14, 2023
@jrha jrha merged commit 842a5c9 into quattor:master Aug 14, 2023
2 checks passed
my $ip = $vip->{ip};
$ip_list->{ip} = $ip;
$ip_list->{'prefix-length'} = get_masklen($self, "$ip/$netmask");
$dummy_iface{name} = $iface_name;
Copy link
Member

Choose a reason for hiding this comment

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

you don't have to write ugly perl code ;)

$dummy_iface{'profile-name'} = $iface_name;
$dummy_iface{type} = "dummy";
$dummy_iface{state} = "up";
$dummy_iface{ipv4}->{enabled} = "true";
Copy link
Member

Choose a reason for hiding this comment

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

if you want a yaml boolean out of this, use the yaml constant for true. what does this geneate? the string true incl quotes, that evaluates to true by accident?

my $netmask = $vip->{netmask} || "255.255.255.255";
my $ip = $vip->{ip};
$ip_list->{ip} = $ip;
$ip_list->{'prefix-length'} = get_masklen($self, "$ip/$netmask");
Copy link
Member

Choose a reason for hiding this comment

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

$self->get_masklen(...)

$dummy_iface{state} = "up";
$dummy_iface{ipv4}->{enabled} = "true";
$dummy_iface{ipv4}->{address} = [$ip_list];
my $iface_cfg->{'interfaces'} = [\%dummy_iface];
Copy link
Member

Choose a reason for hiding this comment

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

the single quotes are not needed when the key is unambigious, so here is. (profile-name isn't, perl thinks you are subtracting 2 function calls in that case)

so you can do

my $iface_cfg->{interfaces} = [{
  'profile-name' => $iface_name,
  type -> "dummy",
  state => "up",
  ipv4 => {
      enabled => $YTRUE,
      address => [$iplist],
  },
}]

Copy link
Member

Choose a reason for hiding this comment

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

@aka7 can you fix this in some other PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, will go through all your comments and will do a new pr to fix all that. np.
Thanks for you review @stdweird appreciate it.

my $vips = $net->{vips};
my $idx = 0;
foreach my $name (sort keys %$vips) {
my $dummy_name="dummy$idx";
Copy link
Member

Choose a reason for hiding this comment

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

spaces around =

but euhm, all dummy vip names might changes by adding a single interface with name that is earlier in alphabetic order? (i have no easy fix besides chagin the schema to a list, but just be aware of this "feature")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants