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

fix: inplace update failed due to virtual kubelet in PUB #1359

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/control/pubcontrol/pub_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (c *commonControl) IsPodStateConsistent(pod *corev1.Pod) bool {
}

// whether other containers is consistent
if err := inplaceupdate.DefaultCheckInPlaceUpdateCompleted(pod); err != nil {
if err := inplaceupdate.DefaultCheckInPlaceUpdateCompleted(pod); err != nil && !shouldSkipCheckPUBCompleted(c.Client, pod) {
klog.V(5).Infof("check pod(%s/%s) InPlaceUpdate failed: %s", pod.Namespace, pod.Name, err.Error())
return false
}
Expand Down
35 changes: 35 additions & 0 deletions pkg/control/pubcontrol/pub_control_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@

import (
"context"
"encoding/json"
"fmt"
"strings"
"time"

appspub "github.com/openkruise/kruise/apis/apps/pub"
policyv1alpha1 "github.com/openkruise/kruise/apis/policy/v1alpha1"
kubeClient "github.com/openkruise/kruise/pkg/client"
"github.com/openkruise/kruise/pkg/util"
Expand All @@ -43,6 +45,11 @@
MaxUnavailablePodSize = 2000
)

var (
Copy link
Member

Choose a reason for hiding this comment

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

How about making var together?

var (
labelNodeTypeKey = "type"
virtualNodeType = "virtual-kubelet"

taintVirtualToFind = &corev1.Taint{
		Key:    "virtual-kubelet.io/provider",
		Effect: corev1.TaintEffectNoSchedule,
	}
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

labelNodeTypeKey = "type"
virtualNodeType = "virtual-kubelet"
)

var ConflictRetry = wait.Backoff{
Steps: 4,
Duration: 500 * time.Millisecond,
Expand Down Expand Up @@ -175,13 +182,26 @@
return true, "", nil
}

// isVirtualKubeletNode Determine whether the node is virtual kubelet
func isVirtualKubeletNode(cli client.Client, nodeName string) bool {
if len(nodeName) == 0 {
return false
}
node := &corev1.Node{}
if err := cli.Get(context.Background(), client.ObjectKey{Name: nodeName}, node); err != nil {
klog.Errorf("get node(%s) failed: %s", nodeName, err.Error())
return false
}
return node.Labels[labelNodeTypeKey] == virtualNodeType
}

func checkAndDecrement(podName string, pub *policyv1alpha1.PodUnavailableBudget, operation policyv1alpha1.PubOperation) error {
if pub.Status.UnavailableAllowed <= 0 {

Check failure on line 199 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: client

Check failure on line 199 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: client

Check failure on line 199 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: client

Check failure on line 199 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / unit-tests

undefined: client
return errors.NewForbidden(policyv1alpha1.Resource("podunavailablebudget"), pub.Name, fmt.Errorf("pub unavailable allowed is negative"))
}
if len(pub.Status.DisruptedPods)+len(pub.Status.UnavailablePods) > MaxUnavailablePodSize {
return errors.NewForbidden(policyv1alpha1.Resource("podunavailablebudget"), pub.Name, fmt.Errorf("DisruptedPods and UnavailablePods map too big - too many unavailable not confirmed by PUB controller"))
}

Check failure on line 204 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: client

Check failure on line 204 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / golangci-lint

undefined: client

Check failure on line 204 in pkg/control/pubcontrol/pub_control_utils.go

View workflow job for this annotation

GitHub Actions / unit-tests

undefined: client

pub.Status.UnavailableAllowed--

Expand Down Expand Up @@ -233,3 +253,18 @@
operations := sets.NewString(strings.Split(operationValue, ",")...)
return operations.Has(string(operation))
}

// shouldSkipCheckPUBCompleted Determine whether the pod should skip check PUB completed
// if the pod is running on a virtual kubelet node and enable UpdateEnvFromMetadata, do not check inplace update state
func shouldSkipCheckPUBCompleted(cli client.Client, pod *corev1.Pod) bool {
if !isVirtualKubeletNode(cli, pod.Spec.NodeName) {
return false
}
inPlaceUpdateState := appspub.InPlaceUpdateState{}
if stateStr, ok := appspub.GetInPlaceUpdateState(pod); !ok {
return false
} else if err := json.Unmarshal([]byte(stateStr), &inPlaceUpdateState); err != nil {
return false
}
return inPlaceUpdateState.UpdateEnvFromMetadata
}
109 changes: 109 additions & 0 deletions pkg/control/pubcontrol/pub_control_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,25 @@ var (
},
},
}

vkNode = &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "virtual-kubelet",
Labels: map[string]string{
"type": "virtual-kubelet",
},
},
}

nonVkNode = &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "not-virtual-kubelet",
Labels: map[string]string{
"ack.aliyun.com": "xxx",
},
},
Spec: corev1.NodeSpec{},
}
)

func TestPodUnavailableBudgetValidatePod(t *testing.T) {
Expand Down Expand Up @@ -398,3 +417,93 @@ func TestGetPodUnavailableBudgetForPod(t *testing.T) {
})
}
}

func Test_isVirtualKubeletNode(t *testing.T) {
cases := []struct {
name string
node *corev1.Node
want bool
}{
{
name: "virtual kubelet node",
node: vkNode,
want: true,
},
{
name: "not virtual kubelet node",
node: nonVkNode,
want: false,
},
}

for _, cs := range cases {
t.Run(cs.name, func(t *testing.T) {
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cs.node).Build()
got := isVirtualKubeletNode(fakeClient, cs.node.Name)
if got != cs.want {
t.Fatalf("isVirtualKubeletNode failed")
}
})
}
}

func Test_shouldSkipCheckPUBCompleted(t *testing.T) {
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vkNode, nonVkNode).Build()
cases := []struct {
name string
pod *corev1.Pod
want bool
}{
{
name: "running on virtual kubelet node with enable UpdateEnvFromMetadata",
pod: &corev1.Pod{
Spec: corev1.PodSpec{
NodeName: "virtual-kubelet",
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: `{"updateEnvFromMetadata":true}`,
},
},
},
want: true,
},
{
name: "running on virtual kubelet node with disable UpdateEnvFromMetadata",
pod: &corev1.Pod{
Spec: corev1.PodSpec{
NodeName: "virtual-kubelet",
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: `{"updateEnvFromMetadata":false}`,
},
},
},
want: false,
},
{
name: "not running on virtual kubelet node with enable UpdateEnvFromMetadata",
pod: &corev1.Pod{
Spec: corev1.PodSpec{
NodeName: "not-virtual-kubelet",
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
appspub.InPlaceUpdateStateKey: `{"updateEnvFromMetadata":true}`,
},
},
},
want: false,
},
}

for _, cs := range cases {
t.Run(cs.name, func(t *testing.T) {
got := shouldSkipCheckPUBCompleted(fakeClient, cs.pod)
if got != cs.want {
t.Fatalf("shouldSkipCheckPUBCompleted failed")
}
})
}
}
Loading