Skip to content

Commit

Permalink
1.分离 client 中的真实和 mock 调用 k8s api,以便于 ut 不依赖于当前的 k8s 环境。
Browse files Browse the repository at this point in the history
factory.go: 将client的实例管理逻辑统一,方便调用和切换fake。兼容当前 KubeClient 全局变量的使用。
k8s_client.go: 统一实现 k8s 真实调用行为, 记录调用日志,去掉全局变量。

2.修改 util 和 util_test 中的相关调用, 统一 util_test 中测试用例的命名风格。
本次修改仅关注于 real 和 mock 分离,后续将完善更多 ut 测试,逐步替换当前 client 中的直接调用方式。

3.Fix UT not be properly executed during CI phase (#911)

4.兼容当前新老调用 KubeClient 的用法,特别是 fake 替换时

Signed-off-by: yangshiqi <[email protected]>
  • Loading branch information
yangshiqi committed Mar 6, 2025
1 parent 857ee57 commit 73454a2
Show file tree
Hide file tree
Showing 14 changed files with 344 additions and 64 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ libvgpu.so
vendor
license
vgpuvalidator
_output/coverage/coverage_pkg.txt
_output/
coverage.out
.DS_Store
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ clean:
$(GO) clean -r -x ./cmd/...
-rm -rf $(OUTPUT_DIR)

.PHONY: all build docker clean $(CMDS)
.PHONY: all build docker clean test $(CMDS)

test:
mkdir -p ./_output/coverage/
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/prometheus/client_golang v1.18.0
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.8.1
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
github.com/urfave/cli/v2 v2.27.1
golang.org/x/net v0.35.0
Expand All @@ -26,6 +27,7 @@ require (
google.golang.org/grpc v1.63.2
google.golang.org/protobuf v1.33.0
gopkg.in/yaml.v2 v2.4.0
gopkg.in/yaml.v3 v3.0.1
gotest.tools/v3 v3.5.1
k8s.io/api v0.29.3
k8s.io/apimachinery v0.29.3
Expand Down Expand Up @@ -72,7 +74,6 @@ require (
github.com/prometheus/common v0.48.0 // indirect
github.com/prometheus/procfs v0.13.0 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
golang.org/x/mod v0.17.0 // indirect
Expand All @@ -84,7 +85,6 @@ require (
google.golang.org/appengine v1.6.8 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240227224415-6ceb2ff114de // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/kube-openapi v0.0.0-20240227032403-f107216b40e2 // indirect
k8s.io/utils v0.0.0-20240102154912-e7106e64919e // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
Expand Down
20 changes: 1 addition & 19 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,6 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ=
golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE=
golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
golang.org/x/net v0.35.0 h1:T5GQRQb2y08kTAByq9L4/bz8cipCdA8FbRTXewonqY8=
golang.org/x/net v0.35.0/go.mod h1:EglIi67kWsHKlRzzVMUD93VMSWGFOMSZgxFjparz1Qk=
golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ=
Expand All @@ -189,10 +185,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w=
golang.org/x/sync v0.11.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand All @@ -204,28 +198,16 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws=
golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc=
golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.21.0 h1:WVXCp+/EBEHOj53Rvu+7KiT/iElMrO8ACK16SMZ3jaA=
golang.org/x/term v0.21.0/go.mod h1:ooXLefLobQVslOqselCNF4SxFAaoS6KujMbsGzSDmX0=
golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=
golang.org/x/term v0.29.0 h1:L6pJp37ocefwRRtYPKSWOWzOtWSxVajvz2ldH/xi3iU=
golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ=
golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI=
golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM=
golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY=
golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
Expand Down
14 changes: 5 additions & 9 deletions pkg/device/cambricon/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,18 @@ func (dev *CambriconDevices) ReleaseNodeLock(n *corev1.Node, p *corev1.Pod) erro
return nil
}

patchData := []byte(`[
{
"op": "remove",
"path": "/metadata/annotations/cambricon.com~1dsmlu.lock"
}
]`)

_, err := client.GetClient().CoreV1().Nodes().Patch(context.TODO(), n.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
newNode := n.DeepCopy()
delete(newNode.ObjectMeta.Annotations, DsmluLockTime)
_, err := client.GetClient().CoreV1().Nodes().Update(context.Background(), newNode, metav1.UpdateOptions{})
for i := 0; i < retry && err != nil; i++ {
klog.ErrorS(err, "Failed to patch node annotation", "node", n.Name, "retry", i)
time.Sleep(time.Duration(rand.Intn(i+1)) * 10 * time.Millisecond)
_, err = client.GetClient().CoreV1().Nodes().Patch(context.TODO(), n.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
_, err = client.GetClient().CoreV1().Nodes().Update(context.Background(), newNode, metav1.UpdateOptions{})

Check warning on line 164 in pkg/device/cambricon/device.go

View check run for this annotation

Codecov / codecov/patch

pkg/device/cambricon/device.go#L164

Added line #L164 was not covered by tests
}
if err != nil {
return fmt.Errorf("releaseNodeLock exceeds retry count %d", retry)
}
delete(n.ObjectMeta.Annotations, DsmluLockTime)
klog.InfoS("Node lock released", "node", n.Name)
return nil
}
Expand Down
11 changes: 2 additions & 9 deletions pkg/device/cambricon/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,17 +480,9 @@ func Test_setNodeLock(t *testing.T) {
},
expectErr: false,
},
{
name: "no node name",
node: corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{},
},
},
expectErr: false,
},
}

client.KubeClient = fake.NewSimpleClientset()
k8sClient := client.GetClient()
if k8sClient != nil {

Expand Down Expand Up @@ -624,6 +616,7 @@ func Test_LockNode(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
node, pod, teardown, clientset := setupTest(t)
client.KubeClient = clientset
defer teardown()

// Set up the node with the specified annotations.
Expand Down
16 changes: 16 additions & 0 deletions pkg/device/devices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,21 @@ func createVNPUConfigs() []ascend.VNPUConfig {
{Name: "vir16", Memory: 17476, AICore: 16},
},
},
{
ChipName: "910B2",
CommonWord: "Ascend910B2",
ResourceName: "huawei.com/Ascend910B2",
ResourceMemoryName: "huawei.com/Ascend910B2-memory",
MemoryAllocatable: 65536,
MemoryCapacity: 65536,
AICore: 24,
AICPU: 6,
Templates: []ascend.Template{
{Name: "vir03_1c_8g", Memory: 8192, AICore: 3, AICPU: 1},
{Name: "vir06_1c_16g", Memory: 16384, AICore: 6, AICPU: 1},
{Name: "vir12_3c_32g", Memory: 32768, AICore: 12, AICPU: 3},
},
},
{
ChipName: "910B3",
CommonWord: "Ascend910B",
Expand Down Expand Up @@ -329,6 +344,7 @@ func setupTest(t *testing.T) (map[string]string, map[string]Devices) {
iluvatar.IluvatarGPUDevice: iluvatar.IluvatarGPUCommonWord,
mthreads.MthreadsGPUDevice: mthreads.MthreadsGPUCommonWord,
metax.MetaxGPUDevice: metax.MetaxGPUCommonWord,
metax.MetaxSGPUDevice: metax.MetaxSGPUCommonWord,
}

return expectedDevices, devicesMap
Expand Down
2 changes: 2 additions & 0 deletions pkg/device/hygon/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"gotest.tools/v3/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"

"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -673,6 +674,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
}

func Test_NodeCleanUp(t *testing.T) {
client.KubeClient = fake.NewSimpleClientset()
tests := []struct {
name string
args string
Expand Down
12 changes: 9 additions & 3 deletions pkg/util/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,18 @@ var (
once sync.Once
)

func init() {
KubeClient = nil
}

func GetClient() kubernetes.Interface {
once.Do(func() {
var err error
KubeClient, err = newClient()
if err != nil {
klog.Fatalf("Failed to create Kubernetes client: %v", err)
if KubeClient == nil {
KubeClient, err = newClient()
if err != nil {
klog.Fatalf("Failed to create Kubernetes client: %v", err)
}

Check warning on line 47 in pkg/util/client/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/client/client.go#L46-L47

Added lines #L46 - L47 were not covered by tests
}
})
return KubeClient
Expand Down
88 changes: 88 additions & 0 deletions pkg/util/client/factory.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
Copyright 2024 The HAMi Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package client

import (
"sync"

"k8s.io/client-go/kubernetes/fake"
)

// KubeClientFactory is a factory class (singleton pattern).
type KubeClientFactory struct {
client KubeInterface
}

var factoryOnce sync.Once

// GetFactory gets the singleton factory object.
// Uses closure to maintain instance as a local variable.
var GetFactory = (func() func() *KubeClientFactory {
var instance *KubeClientFactory

return func() *KubeClientFactory {
factoryOnce.Do(func() {
instance = &KubeClientFactory{}
instance.SetReal() // Use the real client by default.
})
return instance

Check warning on line 42 in pkg/util/client/factory.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/client/factory.go#L38-L42

Added lines #L38 - L42 were not covered by tests
}
})()

// NewInstance directly gets the Kubernetes client instance.
func NewInstance() KubeInterface {
return GetFactory().GetClient()

Check warning on line 48 in pkg/util/client/factory.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/client/factory.go#L47-L48

Added lines #L47 - L48 were not covered by tests
}

func (f *KubeClientFactory) GetClient() KubeInterface {
//TODO: For compatibility with other direct "KubeClient" assignment call points, this line needs to be removed after replacement.
if KubeClient == nil {
f.client = &K8sClient{
client: GetK8sClient().client,
}
KubeClient = GetK8sClient().client
} else {
f.client = &K8sClient{
client: KubeClient,
}
}
return f.client

Check warning on line 63 in pkg/util/client/factory.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/client/factory.go#L51-L63

Added lines #L51 - L63 were not covered by tests
}

func (f *KubeClientFactory) SetFake() *KubeClientFactory {
f.client = &K8sClient{
client: fake.NewSimpleClientset(),
}
//TODO: For compatibility with other direct "KubeClient" assignment call points, this line needs to be removed after replacement.
KubeClient = fake.NewSimpleClientset()
return f

Check warning on line 72 in pkg/util/client/factory.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/client/factory.go#L66-L72

Added lines #L66 - L72 were not covered by tests
}

func (f *KubeClientFactory) SetReal() *KubeClientFactory {
//TODO: For compatibility with other direct "KubeClient" assignment call points, this line needs to be removed after replacement.
if KubeClient == nil {
f.client = &K8sClient{
client: GetK8sClient().client,
}
KubeClient = GetK8sClient().client
} else {
f.client = &K8sClient{
client: KubeClient,
}
}
return f

Check warning on line 87 in pkg/util/client/factory.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/client/factory.go#L75-L87

Added lines #L75 - L87 were not covered by tests
}
34 changes: 34 additions & 0 deletions pkg/util/client/interface.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
Copyright 2024 The HAMi Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package client

import (
"context"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
)

type KubeInterface interface {
GetNode(ctx context.Context, name string, opts metav1.GetOptions) (*corev1.Node, error)
ListPods(ctx context.Context, namespace string, opts metav1.ListOptions) (*corev1.PodList, error)
GetPod(ctx context.Context, namespace, name string, opts metav1.GetOptions) (*corev1.Pod, error)
PatchNode(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions) (*corev1.Node, error)
PatchPod(ctx context.Context, namespace string, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions) (*corev1.Pod, error)
CreateNode(ctx context.Context, node *corev1.Node, opts metav1.CreateOptions) (*corev1.Node, error)
}
Loading

0 comments on commit 73454a2

Please sign in to comment.