[WIP]SKS-1825: Use the default IPPool as the global IPPool #37

haijianyang · 2024-03-22T07:00:31Z

Issue

CAPE 增加了网卡级别的网络配置后：按照 CAPI IPAM 社区现在的规范，IPAM 需要提供两种作用域的 IPPool：

一种需要指定 namespace，只能给同样命名空间的 ElfMachine 使用。

type NetworkDeviceSpec struct {
	IPAddrs []string `json:"ipAddrs,omitempty"`
	AddressesFromPools []corev1.TypedLocalObjectReference `json:"addressesFromPools,omitempty"`
}

type TypedLocalObjectReference struct {
	// APIGroup is the group for the resource being referenced.
	// If APIGroup is not specified, the specified Kind must be in the core API group.
	// For any other third-party types, APIGroup is required.
	// +optional
	APIGroup *string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`
	// Kind is the type of resource being referenced
	Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
	// Name is the name of resource being referenced
	Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
}

另一种不需要 namespace，可以给所有的命名空间的 ElfMachine 共享使用。

当前 Metal3 只提供了 namespace 作用域的 IPPool。

目前 SKS 的集群只使用了 default 命名空间，所以这种情况下，所有的 Machine 都在 default 命名空间，所以都可以使用到 default 命名空间的 IPPool。

但 SKS E2E 每个 case 都使用了不同的 namespace，例如 sks-e2e-main-43-k8s-mhc、sks-e2e-main-43-k8s-scale-groups。这样导致 E2E 创建的 IPPool 不能被所有的 cases 的 Machine 使用。

type ObjectKey struct {
	// name is unique within a namespace to reference a object resource.
	// +kubebuilder:validation:MinLength=1
	Name string `json:"name"`
	// namespace defines the space within which the object name must be unique.
	// +optional
	Namespace string `json:"namespace,omitempty"`
}

Change

选择 IPPool 的流程调整：
(1)如果设置了 ip-pool-name：

如果 elfMachine 设置了 AddressesFromPool，会使用 AddressesFromPool.name 作为
cluster.x-k8s.io/ip-pool-name，对应的 elfMachine.namespace 作为 cluster.x-k8s.io/ip-pool-namespace（这部分为了支持在 Device 配置 IPPool 加的，其他的 2-3 部分逻辑是原来的）
否则会从 elfMachine 对应的 ElfMachineTemplate 获取
cluster.x-k8s.io/ip-pool-namespace 和 cluster.x-k8s.io/ip-pool-name（SKS 从 KSC 取值设置）
使用 ip-pool-name 和 ip-pool-namespace 获取 IPPool，能获取到直接返回，获取不到下一步
查询在默认命名空间 default 中的名为 ip-pool-name 的 IPPool

(2)如果没有设置 ip-pool-name：

从 elfMachine.namespace 查询带有 is-default 标签的 IPPool，如果有多个取第一个。如果没有，下一步
从默认命名空间查询所有的查询带有 is-default 标签的 IPPool，如果有多个取第一个。

默认命名空间 cape-system 改成 default

Test

E2E

codecov · 2024-03-22T07:04:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.40%. Comparing base (d5d7b1d) to head (7a77cc9).

❗ Current head 7a77cc9 differs from pull request most recent head b66f9b5. Consider uploading reports for the commit b66f9b5 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #37   +/-   ##
=======================================
  Coverage   68.40%   68.40%           
=======================================
  Files           2        2           
  Lines         307      307           
=======================================
  Hits          210      210           
  Misses         81       81           
  Partials       16       16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jessehu · 2024-03-22T15:48:45Z

pkg/ipam/metal3io/ipam.go

 	ipPool = ipPoolList.Items[0]
+	for i := 0; i < len(ipPoolList.Items); i++ {
+		if ipPoolList.Items[i].Name == poolName {


@haijianyang 这个PR的改动是为了解决e2e-test中给ZBS CSI用的第二块网卡使用静态IP时如何指定IPPool的问题。当在network.devices[1]中不指定addressesFromPools时，因为有2个 cape-system/IPPool，所以不知道应该使用哪一个；如果指定了IPPool name，又默认是和ElfMachine在同一个namespace，而匹配不到cape-system中的IPPool.

当前选择的方案1是可行的。只是"ippool.cluster.x-k8s.io/is-default": true的IPPool在同一个namespace中应该只能有一个，它是给没有指定IPPool name的device使用的；如果指定了IPPool name，就在当前namespace或默认namespace cape-system中匹配同名的IPPool。这样是否合理？在代码中也需要加上一些必要注释来说明，不然不太容易看懂代码，也不知道代码写得对不对，需要看PR描述才能弄明白代码的逻辑。
另外：默认namespace cape-system 是否应该调整为default?

spec: template: spec: cloneMode: FastClone diskGiB: 230 ha: true memoryMiB: 8192 network: devices: - networkType: IPV4_DHCP vlan: dd1f408f-7715-48c1-a817-13c3568f1d93_4cd00407-63ca-440b-80b7-ceacfccb8d08 - addressesFromPools: - apiGroup: ipam.metal3.io kind: IPPool name: ip-pool-e2e-vlan101 networkType: IPV4 vlan: dd1f408f-7715-48c1-a817-13c3568f1d93_55052921-b84f-4884-8a90-d4ba173e7387 nameservers: - 10.255.0.2

按照CAPI IPAM的定义，InCluster 提供了 InClusterIPPool 和 GlobalInClusterIPPool 两种 Pool。InClusterIPPool 只能给和它同一个 namespace 的 Cluster 使用。 GlobalInClusterIPPool (scope=Cluster) 则可以给所有的 namespace 的 Cluster 使用。

在CAPE中使用的 AddressesFromPools []corev1.TypedLocalObjectReference 是需要能指定 Kind(将来是InClusterIPPool或GlobalInClusterIPPool)、并且不用指定namespace的。
在KSC中的 IPPoolRefs []ObjectKey 不能支持Kind，并且其namespace也用不上。这样的话将来支持CAPI IPAM后，还得调整一次KSC CRD定义，并且是不兼容的(一个有namespace，一个没有)。

当前 E2E 的 IPPool 需要跨多个 namespace 使用，且需要使用多个 IPPool。

方法1不是很优雅。是否考虑一步到位，让 KSC 支持 Kind。

改的话，1.2中KSC CR中使用的Spec.NetworkConfig.IPPoolRef *ObjectKey也得改成支持Kind和Name的TypedLocalObjectReference了，前者有namespace，后者没有，需要做migrate，不然在1.3中apply1.2 KSC CR就会出错了。或者是换个类型同时也支持namespace。

并且也先只用支持一种IPPool type。
@haijianyang

或者问题简单一点：现在如何解决 E2E 的 IPPool 需要跨多个 namespace 的使用。生产环境目前不存在跨多个 namespace 的 IPPool 的需求。

在我的第一个comment中已经说了方案

“如果指定了IPPool name，就在当前namespace或默认namespace cape-system中匹配同名的IPPool。这样是否合理？”

评论已经说了方法 1 不合理了。

要不就改了 E2E 使用多个 namespace。要不然代码就会很奇怪，现在改的代码是只是为了 E2E。

当前选择的方案1是可行的。

只是"ippool.cluster.x-k8s.io/is-default": true的IPPool在同一个namespace中应该只能有一个，它是给没有指定IPPool name的device使用的；

如果没有指定IPPool name，就去找默认IPPool。

如果指定了IPPool name，就在当前namespace或默认namespace cape-system中匹配同名的IPPool。在代码中也需要加上一些必要注释来说明，不然不太容易看懂代码，也不知道代码写得对不对，需要看PR描述才能弄明白代码的逻辑。

另外：默认namespace cape-system 是否应该调整为default?

(1)如果设置了 ip-pool-name：

如果 elfMachine 设置了 AddressesFromPool，会使用 AddressesFromPool.name 作为
cluster.x-k8s.io/ip-pool-name，对应的 elfMachine.namespace 作为 cluster.x-k8s.io/ip-pool-namespace（这部分为了支持在 Device 配置 IPPool 加的，其他的 2-3 部分逻辑是原来的）

否则会从 elfMachine 对应的 ElfMachineTemplate 获取
cluster.x-k8s.io/ip-pool-namespace 和 cluster.x-k8s.io/ip-pool-name（SKS 从 KSC 取值设置）

使用 ip-pool-name 和 ip-pool-namespace 获取 IPPool，能获取到直接返回，获取不到下一步

查询在默认命名空间 default 中的名为 ip-pool-name 的 IPPool

(2)如果没有设置 ip-pool-name：

从 elfMachine.namespace 查询带有 is-default 标签的 IPPool，如果有多个取第一个。如果没有，下一步

从默认命名空间查询所有的查询带有 is-default 标签的 IPPool，如果有多个取第一个。

jessehu · 2024-03-29T08:02:06Z

pkg/ipam/metal3io/ipam.go

 	if err := m.List(
 		ctx,
 		ipPoolList,
-		client.InNamespace(poolNamespace),
+		client.InNamespace(ipam.DefaultIPPoolNamespace),


如果DefaultIPPoolNamespace和clusterMeta.Namespace 一样，这个if 是不是就不用执行了？

后面还有 ClusterIPPoolGroupKey & ClusterNetworkNameKey，这两个没有用到，可以考虑删了。

jessehu · 2024-03-29T08:04:33Z

pkg/ipam/metal3io/ipam.go

+	// 2. Otherwise use default ip-pool
+
+	ipPoolList := &ipamv1.IPPoolList{}
+	if err := m.List(


建议这里改成一个func：getIPPoolByLabels()

haijianyang requested a review from jessehu March 22, 2024 07:00

jessehu reviewed Mar 22, 2024

View reviewed changes

Refactor IPPool selection code

302431d

haijianyang force-pushed the golbal branch from 1196d4c to 302431d Compare March 29, 2024 07:07

haijianyang requested a review from jessehu March 29, 2024 07:13

jessehu reviewed Mar 29, 2024

View reviewed changes

jessehu approved these changes Apr 2, 2024

View reviewed changes

haijianyang added 2 commits April 2, 2024 02:03

test

7a77cc9

Add unit tests

b66f9b5

haijianyang merged commit 9616ec2 into smartxworks:main Apr 9, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]SKS-1825: Use the default IPPool as the global IPPool #37

[WIP]SKS-1825: Use the default IPPool as the global IPPool #37

haijianyang commented Mar 22, 2024 •

edited

Loading

codecov bot commented Mar 22, 2024 •

edited

Loading

jessehu Mar 22, 2024

jessehu Mar 22, 2024

haijianyang Mar 25, 2024

jessehu Mar 26, 2024

haijianyang Mar 26, 2024 •

edited

Loading

jessehu Mar 26, 2024

haijianyang Mar 26, 2024

jessehu Mar 26, 2024

haijianyang Mar 26, 2024 •

edited

Loading

jessehu Mar 29, 2024

haijianyang Apr 1, 2024

jessehu Mar 29, 2024

[WIP]SKS-1825: Use the default IPPool as the global IPPool #37

[WIP]SKS-1825: Use the default IPPool as the global IPPool #37

Conversation

haijianyang commented Mar 22, 2024 • edited Loading

Issue

Change

Test

codecov bot commented Mar 22, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haijianyang Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haijianyang Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haijianyang commented Mar 22, 2024 •

edited

Loading

codecov bot commented Mar 22, 2024 •

edited

Loading

haijianyang Mar 26, 2024 •

edited

Loading

haijianyang Mar 26, 2024 •

edited

Loading