singlepointipam can work with IP ranges

[zolug]

Related issue: cmd-nse-remote-vlan/110

Signed-off-by: Lugossy Zoltan [email protected]

Description

ippool extended so that IP addresses from an IP pool could be added to the pool.
singlepointipam modified to also support allocating IPs based on IP ranges.

Issue link

networkservicemesh/cmd-nse-remote-vlan#110

How Has This Been Tested?

• Added unit testing to cover
• Tested manually
• Tested by integration testing
• Have not tested

Types of changes

• Bug fix
• New functionallity
• Documentation
• Refactoring
• CI

[denis-tingaikin]

@zolug if I read the code correctly, basically these changes are needed to pre-exclude some of the IP addresses from the ippool for the connection.

Does it look correct?

[denis-tingaikin]

Question: Is it possible to use https://github.com/networkservicemesh/api/blob/main/pkg/api/networkservice/connectioncontext.proto#L69 to solve the problem?

See at chain element https://github.com/networkservicemesh/sdk/tree/main/pkg/networkservice/common/excludedprefixes

[zolug]

@zolug if I read the code correctly, basically these changes are needed to pre-exclude some of the IP addresses from the ippool for the connection.

Does it look correct?

@denis-tingaikin Basically yes, but the goal is to split up a particular network between multiple independently operating singlepointipams.

[denis-tingaikin]

@zolug OK, now I see the motivation.

So, now let's figure out if is it possible to re-use https://github.com/networkservicemesh/sdk/tree/main/pkg/networkservice/common/excludedprefixes chain element.

[zolug]

@zolug OK, now I see the motivation.

So, now let's figure out if is it possible to re-use https://github.com/networkservicemesh/sdk/tree/main/pkg/networkservice/common/excludedprefixes chain element.

I'll try to list my points, but I don't think that's really suitable. Please feel free to weigh in and correct me.

-It's not just about excluding a few IP addresses but splitting up networks into ranges to be used by multiple independent singlepointipams or VLAN Remote NSEs. This could mean many-many excluded IPs for example (unless cleverly describing them with multiple networks).
-Also, for me it seems that the code dealing with excluded prefixes neither suport ranges.
And adding excluded ranges support seems way more intrusive. And could also imply that compared to the proposed PR a simple NSE uplift would not be sufficient to introduce the functionality (this might not be a priority for us though, but nsm core is handled separately and in general we try to avoid upgrade requirements towards it).
-A common excluded prefixes config in nsmgr is not feasible as you can see. Which IMHO leaves room for an NSC or NSE based excluded prefixes approach.
-It would be also preferable to contain the ipam configuration related information in one place. Which the excluded prefixes concept seems to contradict if these prefixes are passed by the NSCs or sg else (other than the NSE) on the path.
-I guess some NSE based excluded prefixes solution could provide config collocation. Where for example a new chain would "amend" the incoming request. Then once the ipam chain had finished it would remove the additional excluded prexies it previously added. Thus basically limiting the scope of the changes required in NSM (e.g. avoid impact on nsmgr merge). But this approach feels hackis and not generic either.
-We would like to offer a clean and simple configuration option for our users. As the goal is to split up a specific subnetwork among multiple independent IPAMs, the allowed ranges approach looks more natural. And I have a feeling that it could be overwhelming and error prone to require users to configure excluded prefixes instead of allowed ranges. But I could be completely wrong here :)
Of course doing the conversion internally in our product could be also an option. We do have an Operator as the official deployment option. However in case of helm based deployments it could be tricky/cumbersome to implement such allowed ranges to exluded IPs mapping.

So basically it felt more natural to modify the IPAM prefix configuration itself, both because it seems to have limited impact on NSM and the configuration feels more straightforward.

[denis-tingaikin]

I think it can be merged.

Would you like to add an encoder for IP ranges for envoyconfig to the sdk/pkg/tools?

[edwarnicke]

Specifically, do you want to add something like encoding.TextUnMarshaller so envconfig can unmarshal for you automatically:

https://github.com/kelseyhightower/envconfig#supported-struct-field-types

[denis-tingaikin]

@zolug Do you have any updates on this?