Skip to content

Commit

Permalink
simplify the addon progressing message
Browse files Browse the repository at this point in the history
Signed-off-by: haoqing0110 <[email protected]>
  • Loading branch information
haoqing0110 committed Jun 6, 2024
1 parent 61a74bb commit e0faf37
Show file tree
Hide file tree
Showing 23 changed files with 153 additions and 206 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ metadata:
categories: Integration & Delivery,OpenShift Optional
certified: "false"
containerImage: quay.io/open-cluster-management/registration-operator:latest
createdAt: "2024-05-22T01:34:44Z"
createdAt: "2024-06-06T06:12:51Z"
description: Manages the installation and upgrade of the ClusterManager.
operators.operatorframework.io/builder: operator-sdk-v1.32.0
operators.operatorframework.io/builder: operator-sdk-v1.28.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
repository: https://github.com/open-cluster-management-io/ocm
support: Open Cluster Management Community
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ annotations:
operators.operatorframework.io.bundle.package.v1: cluster-manager
operators.operatorframework.io.bundle.channels.v1: stable
operators.operatorframework.io.bundle.channel.default.v1: stable
operators.operatorframework.io.metrics.builder: operator-sdk-v1.32.0
operators.operatorframework.io.metrics.builder: operator-sdk-v1.28.0
operators.operatorframework.io.metrics.mediatype.v1: metrics+v1
operators.operatorframework.io.metrics.project_layout: go.kubebuilder.io/v3
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ metadata:
categories: Integration & Delivery,OpenShift Optional
certified: "false"
containerImage: quay.io/open-cluster-management/registration-operator:latest
createdAt: "2024-05-22T01:34:44Z"
createdAt: "2024-06-06T06:12:51Z"
description: Manages the installation and upgrade of the Klusterlet.
operators.operatorframework.io/builder: operator-sdk-v1.32.0
operators.operatorframework.io/builder: operator-sdk-v1.28.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
repository: https://github.com/open-cluster-management-io/ocm
support: Open Cluster Management Community
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ annotations:
operators.operatorframework.io.bundle.package.v1: klusterlet
operators.operatorframework.io.bundle.channels.v1: stable
operators.operatorframework.io.bundle.channel.default.v1: stable
operators.operatorframework.io.metrics.builder: operator-sdk-v1.32.0
operators.operatorframework.io.metrics.builder: operator-sdk-v1.28.0
operators.operatorframework.io.metrics.mediatype.v1: metrics+v1
operators.operatorframework.io.metrics.project_layout: go.kubebuilder.io/v3
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ require (
k8s.io/kube-aggregator v0.29.3
k8s.io/utils v0.0.0-20240310230437-4693a0247e57
open-cluster-management.io/addon-framework v0.9.1-0.20240419070222-e703fc5a2556
open-cluster-management.io/api v0.13.1-0.20240521030453-9d94703b9eba
open-cluster-management.io/api v0.13.1-0.20240605083248-f9e7f50520fc
open-cluster-management.io/sdk-go v0.13.1-0.20240605055850-874c2c7ac523
sigs.k8s.io/controller-runtime v0.17.3
sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,8 @@ k8s.io/utils v0.0.0-20240310230437-4693a0247e57 h1:gbqbevonBh57eILzModw6mrkbwM0g
k8s.io/utils v0.0.0-20240310230437-4693a0247e57/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
open-cluster-management.io/addon-framework v0.9.1-0.20240419070222-e703fc5a2556 h1:X3vJEx9agC94l7SitpWZFDshISdL1niqVH0+diyqfJo=
open-cluster-management.io/addon-framework v0.9.1-0.20240419070222-e703fc5a2556/go.mod h1:HayKCznnlyW+0dUJQGj5sNR6i3tvylSySD3YnvZkBtY=
open-cluster-management.io/api v0.13.1-0.20240521030453-9d94703b9eba h1:UsXnD4/N7pxYupPgoLvTq8wO73V72vD2D2ZkDd4iws0=
open-cluster-management.io/api v0.13.1-0.20240521030453-9d94703b9eba/go.mod h1:yrNuMMpciXjXPnj2yznb6LTyrGliiTrFZAJDp/Ck3c4=
open-cluster-management.io/api v0.13.1-0.20240605083248-f9e7f50520fc h1:tcfncubZRFphYtDXBE7ApBNlSnj1RNazhW+8F01XYYg=
open-cluster-management.io/api v0.13.1-0.20240605083248-f9e7f50520fc/go.mod h1:ltijKJhDifrPH0csvCUmFt5lzaERv+BBfh6X3l83rT0=
open-cluster-management.io/sdk-go v0.13.1-0.20240605055850-874c2c7ac523 h1:XMtsnv0zT8UvXcH4JbqquzhK4BK/XrCg81pCmp50VDs=
open-cluster-management.io/sdk-go v0.13.1-0.20240605055850-874c2c7ac523/go.mod h1:muWzHWsgK8IsopltwTnsBjf4DN9IcC9rF0G2uEq/Pjw=
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.28.0 h1:TgtAeesdhpm2SGwkQasmbeqDo8th5wOBA5h/AjTKA4I=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,14 @@ spec:
server side apply with work-controller as the field
manager. If there is conflict, the related Applied
condition of manifest will be in the status of False
with the reason of ApplyConflict.
with the reason of ApplyConflict. ReadOnly type means
the agent will only check the existence of the resource
based on its metadata.
enum:
- Update
- CreateOnly
- ServerSideApply
- ReadOnly
type: string
required:
- type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,14 @@ spec:
means to update resource using server side apply with
work-controller as the field manager. If there is conflict,
the related Applied condition of manifest will be in the
status of False with the reason of ApplyConflict.
status of False with the reason of ApplyConflict. ReadOnly
type means the agent will only check the existence of
the resource based on its metadata.
enum:
- Update
- CreateOnly
- ServerSideApply
- ReadOnly
type: string
required:
- type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,14 @@ spec:
server side apply with work-controller as the field
manager. If there is conflict, the related Applied
condition of manifest will be in the status of False
with the reason of ApplyConflict.
with the reason of ApplyConflict. ReadOnly type means
the agent will only check the existence of the resource
based on its metadata.
enum:
- Update
- CreateOnly
- ServerSideApply
- ReadOnly
type: string
required:
- type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"

"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
Expand All @@ -31,17 +30,7 @@ func (d *cmaProgressingReconciler) reconcile(
continue
}

isUpgrade := false

for _, configReference := range installProgression.ConfigReferences {
if configReference.LastAppliedConfig != nil {
isUpgrade = true
break
}
}

setAddOnInstallProgressionsAndLastApplied(&cmaCopy.Status.InstallProgressions[i],
isUpgrade,
placementNode.countAddonUpgrading(),
placementNode.countAddonUpgradeSucceed(),
placementNode.countAddonUpgradeFailed(),
Expand All @@ -59,45 +48,23 @@ func (d *cmaProgressingReconciler) reconcile(

func setAddOnInstallProgressionsAndLastApplied(
installProgression *addonv1alpha1.InstallProgression,
isUpgrade bool,
progressing, done, failed, timeout, total int) {
// always update progressing condition when there is no config
// skip update progressing condition when last applied config already the same as desired
skip := len(installProgression.ConfigReferences) > 0
for _, configReference := range installProgression.ConfigReferences {
if !equality.Semantic.DeepEqual(configReference.LastAppliedConfig, configReference.DesiredConfig) &&
!equality.Semantic.DeepEqual(configReference.LastKnownGoodConfig, configReference.DesiredConfig) {
skip = false
}
}
if skip {
return
}

condition := metav1.Condition{
Type: addonv1alpha1.ManagedClusterAddOnConditionProgressing,
}
if (total == 0 && done == 0) || (done != total) {
condition.Status = metav1.ConditionTrue
if isUpgrade {
condition.Reason = addonv1alpha1.ProgressingReasonUpgrading
condition.Message = fmt.Sprintf("%d/%d upgrading..., %d failed %d timeout.", progressing+done, total, failed, timeout)
} else {
condition.Reason = addonv1alpha1.ProgressingReasonInstalling
condition.Message = fmt.Sprintf("%d/%d installing..., %d failed %d timeout.", progressing+done, total, failed, timeout)
}
condition.Reason = addonv1alpha1.ProgressingReasonProgressing
condition.Message = fmt.Sprintf("%d/%d progressing..., %d failed %d timeout.", progressing+done, total, failed, timeout)
} else {
for i, configRef := range installProgression.ConfigReferences {
installProgression.ConfigReferences[i].LastAppliedConfig = configRef.DesiredConfig.DeepCopy()
installProgression.ConfigReferences[i].LastKnownGoodConfig = configRef.DesiredConfig.DeepCopy()
}
condition.Status = metav1.ConditionFalse
if isUpgrade {
condition.Reason = addonv1alpha1.ProgressingReasonUpgradeSucceed
condition.Message = fmt.Sprintf("%d/%d upgrade completed with no errors, %d failed %d timeout.", done, total, failed, timeout)
} else {
condition.Reason = addonv1alpha1.ProgressingReasonInstallSucceed
condition.Message = fmt.Sprintf("%d/%d install completed with no errors, %d failed %d timeout.", done, total, failed, timeout)
}
condition.Reason = addonv1alpha1.ProgressingReasonCompleted
condition.Message = fmt.Sprintf("%d/%d completed with no errors, %d failed %d timeout.", done, total, failed, timeout)
}
meta.SetStatusCondition(&installProgression.Conditions, condition)
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
if cma.Status.InstallProgressions[0].ConfigReferences[0].LastKnownGoodConfig != nil {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonInstalling {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonProgressing {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions[0].Reason)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "0/2 installing..., 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "0/2 progressing..., 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions[0].Message)
}
},
Expand Down Expand Up @@ -185,10 +185,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
if cma.Status.InstallProgressions[0].ConfigReferences[0].LastKnownGoodConfig != nil {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonInstalling {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonProgressing {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions[0].Reason)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/2 installing..., 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/2 progressing..., 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions[0].Message)
}
},
Expand Down Expand Up @@ -268,10 +268,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig) {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonInstallSucceed {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonCompleted {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/1 install completed with no errors, 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/1 completed with no errors, 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
},
Expand Down Expand Up @@ -344,10 +344,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
if cma.Status.InstallProgressions[0].ConfigReferences[0].LastKnownGoodConfig != nil {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonUpgrading {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonProgressing {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/2 upgrading..., 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/2 progressing..., 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
},
Expand Down Expand Up @@ -431,10 +431,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig) {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonUpgradeSucceed {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonCompleted {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/1 upgrade completed with no errors, 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/1 completed with no errors, 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
},
Expand Down Expand Up @@ -517,10 +517,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
if cma.Status.InstallProgressions[0].ConfigReferences[0].LastKnownGoodConfig != nil {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonUpgrading {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonProgressing {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "0/1 upgrading..., 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "0/1 progressing..., 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
},
Expand Down Expand Up @@ -592,10 +592,10 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
if cma.Status.InstallProgressions[0].ConfigReferences[0].LastKnownGoodConfig != nil {
t.Errorf("InstallProgressions LastKnownGoodConfig is not correct: %v", cma.Status.InstallProgressions[0].ConfigReferences[0])
}
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonInstalling {
if cma.Status.InstallProgressions[0].Conditions[0].Reason != addonv1alpha1.ProgressingReasonProgressing {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions)
}
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/2 installing..., 0 failed 0 timeout." {
if cma.Status.InstallProgressions[0].Conditions[0].Message != "1/2 progressing..., 0 failed 0 timeout." {
t.Errorf("InstallProgressions condition is not correct: %v", cma.Status.InstallProgressions[0].Conditions[0].Message)
}
},
Expand Down
6 changes: 3 additions & 3 deletions pkg/addon/controllers/addonconfiguration/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ func (n *addonNode) setRolloutStatus() {
// desired config spec hash matches actual, but last applied config spec hash doesn't match actual
} else if !equality.Semantic.DeepEqual(actual.LastAppliedConfig, actual.DesiredConfig) {
switch progressingCond.Reason {
case addonv1alpha1.ProgressingReasonInstallFailed, addonv1alpha1.ProgressingReasonUpgradeFailed:
case addonv1alpha1.ProgressingReasonFailed:
n.status.Status = clustersdkv1alpha1.Failed
n.status.LastTransitionTime = &progressingCond.LastTransitionTime
case addonv1alpha1.ProgressingReasonInstalling, addonv1alpha1.ProgressingReasonUpgrading:
case addonv1alpha1.ProgressingReasonProgressing:
n.status.Status = clustersdkv1alpha1.Progressing
n.status.LastTransitionTime = &progressingCond.LastTransitionTime
default:
Expand All @@ -96,7 +96,7 @@ func (n *addonNode) setRolloutStatus() {

// succeed
n.status.Status = clustersdkv1alpha1.Succeeded
if progressingCond.Reason == addonv1alpha1.ProgressingReasonInstallSucceed || progressingCond.Reason == addonv1alpha1.ProgressingReasonUpgradeSucceed {
if progressingCond.Reason == addonv1alpha1.ProgressingReasonCompleted {
n.status.LastTransitionTime = &progressingCond.LastTransitionTime
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/addon/controllers/addonconfiguration/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func TestConfigurationGraph(t *testing.T) {
}, []metav1.Condition{
{
Type: addonv1alpha1.ManagedClusterAddOnConditionProgressing,
Reason: addonv1alpha1.ProgressingReasonUpgradeFailed,
Reason: addonv1alpha1.ProgressingReasonFailed,
LastTransitionTime: fakeTime,
},
}),
Expand All @@ -245,7 +245,7 @@ func TestConfigurationGraph(t *testing.T) {
}, []metav1.Condition{
{
Type: addonv1alpha1.ManagedClusterAddOnConditionProgressing,
Reason: addonv1alpha1.ProgressingReasonUpgrading,
Reason: addonv1alpha1.ProgressingReasonProgressing,
LastTransitionTime: fakeTime,
},
}),
Expand All @@ -266,7 +266,7 @@ func TestConfigurationGraph(t *testing.T) {
}, []metav1.Condition{
{
Type: addonv1alpha1.ManagedClusterAddOnConditionProgressing,
Reason: addonv1alpha1.ProgressingReasonUpgradeSucceed,
Reason: addonv1alpha1.ProgressingReasonCompleted,
LastTransitionTime: fakeTime,
},
}),
Expand All @@ -287,7 +287,7 @@ func TestConfigurationGraph(t *testing.T) {
}, []metav1.Condition{
{
Type: addonv1alpha1.ManagedClusterAddOnConditionProgressing,
Reason: addonv1alpha1.ProgressingReasonUpgradeSucceed,
Reason: addonv1alpha1.ProgressingReasonCompleted,
LastTransitionTime: fakeTime,
},
}),
Expand Down
Loading

0 comments on commit e0faf37

Please sign in to comment.