Repository: kubean-io/kubean
Pull request #831 (2023-06-19 03:40:29)

fix spray-job endTime when operator crashed And return pkg_mgr info
<!--  

Thanks for sending a pull request!  Here are some tips for you:

* make sure your commit is signed off

-->

**What type of PR is this?**
<!-- 
Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
-->

/kind bug

**What this PR does / why we need it**:


**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes https://github.com/kubean-io/kubean/issues/826

**Special notes for your reviewer**:



kubean-pr-robot[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubean-io/kubean/pull/831#" title="Author self-approved">hangscer8</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubean-io%2Fkubean).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubean-io/kubean/blob/main/OWNERS)~~ [hangscer8]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/controllers/clusterops/controller.go b/pkg/controllers/clusterops/controller.go
index 656d97803..d2c1783c4 100644
--- a/pkg/controllers/clusterops/controller.go
+++ b/pkg/controllers/clusterops/controller.go
@@ -123,10 +123,10 @@ func (c *Controller) FetchGlobalInfoManifest() (*manifestv1alpha1.Manifest, erro
 	return global, nil
 }
 
-func (c *Controller) UpdateStatusLoop(clusterOps *clusteroperationv1alpha1.ClusterOperation, fetchJobStatus func(*clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, error)) (bool, error) {
+func (c *Controller) UpdateStatusLoop(clusterOps *clusteroperationv1alpha1.ClusterOperation, fetchJobStatus func(*clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, *metav1.Time, error)) (bool, error) {
 	if clusterOps.Status.Status == clusteroperationv1alpha1.RunningStatus || len(clusterOps.Status.Status) == 0 {
 		// need fetch jobStatus again when the last status of job is running
-		jobStatus, err := fetchJobStatus(clusterOps)
+		jobStatus, completionTime, err := fetchJobStatus(clusterOps)
 		if err != nil {
 			return false, err
 		}
@@ -137,6 +137,9 @@ func (c *Controller) UpdateStatusLoop(clusterOps *clusteroperationv1alpha1.Clust
 		// the status  succeed or failed
 		clusterOps.Status.Status = jobStatus
 		clusterOps.Status.EndTime = &metav1.Time{Time: time.Now()}
+		if completionTime != nil {
+			clusterOps.Status.EndTime = completionTime
+		}
 		if err := c.Client.Status().Update(context.Background(), clusterOps); err != nil {
 			return false, err
 		}
@@ -146,29 +149,29 @@ func (c *Controller) UpdateStatusLoop(clusterOps *clusteroperationv1alpha1.Clust
 	return false, nil
 }
 
-func (c *Controller) FetchJobStatus(clusterOps *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, error) {
+func (c *Controller) FetchJobConditionStatusAndCompletionTime(clusterOps *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, *metav1.Time, error) {
 	if clusterOps.Status.JobRef.IsEmpty() {
-		return "", fmt.Errorf("clusterOps %s no job", clusterOps.Name)
+		return "", nil, fmt.Errorf("clusterOps %s no job", clusterOps.Name)
 	}
 	targetJob, err := c.ClientSet.BatchV1().Jobs(clusterOps.Status.JobRef.NameSpace).Get(context.Background(), clusterOps.Status.JobRef.Name, metav1.GetOptions{})
 	if apierrors.IsNotFound(err) {
 		// maybe the job is removed.
 		klog.Errorf("clusterOps %s  job %s not found", clusterOps.Name, clusterOps.Status.JobRef.Name)
-		return clusteroperationv1alpha1.FailedStatus, nil
+		return clusteroperationv1alpha1.FailedStatus, nil, nil
 	}
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
 	// according to the job condtions, return success or failed
 	for _, contion := range targetJob.Status.Conditions {
 		if contion.Type == batchv1.JobComplete && contion.Status == corev1.ConditionTrue {
-			return clusteroperationv1alpha1.SucceededStatus, nil
+			return clusteroperationv1alpha1.SucceededStatus, targetJob.Status.CompletionTime, nil
 		} else if contion.Type == batchv1.JobFailed && contion.Status == corev1.ConditionTrue {
-			return clusteroperationv1alpha1.FailedStatus, nil
+			return clusteroperationv1alpha1.FailedStatus, targetJob.Status.CompletionTime, nil
 		}
 	}
 
-	return clusteroperationv1alpha1.RunningStatus, nil
+	return clusteroperationv1alpha1.RunningStatus, nil, nil
 }
 
 func (c *Controller) ListClusterOps(clusterName string) ([]clusteroperationv1alpha1.ClusterOperation, error) {
@@ -320,7 +323,7 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
 		return controllerruntime.Result{RequeueAfter: RequeueAfter}, nil
 	}
 
-	needRequeue, err = c.UpdateStatusLoop(clusterOps, c.FetchJobStatus)
+	needRequeue, err = c.UpdateStatusLoop(clusterOps, c.FetchJobConditionStatusAndCompletionTime)
 	if err != nil {
 		klog.Error(err)
 		return controllerruntime.Result{RequeueAfter: RequeueAfter}, err
diff --git a/playbooks/precheck.yml b/playbooks/precheck.yml
index 2c23a98bb..b8e197d88 100644
--- a/playbooks/precheck.yml
+++ b/playbooks/precheck.yml
@@ -93,6 +93,7 @@
         ( if [ ! -f /tmp/kubean_data_temp_cache ]; then touch /tmp/kubean_data_temp_cache ; fi )
         yq -i '.{{inventory_hostname}}.os_kernel_version_output="{{ ansible_kernel }}"' /tmp/kubean_data_temp_cache
         yq -i '.{{inventory_hostname}}.os_kernel_version="{{ ansible_kernel.split('-')[0] }}"' /tmp/kubean_data_temp_cache
+        yq -i '.{{inventory_hostname}}.pkg_mgr="{{ ansible_pkg_mgr }}"' /tmp/kubean_data_temp_cache
       ignore_errors: true
       delegate_to: localhost
       when:


Test patch:
diff --git a/pkg/controllers/clusterops/controller_test.go b/pkg/controllers/clusterops/controller_test.go
index a1a539691..5dd87d77c 100644
--- a/pkg/controllers/clusterops/controller_test.go
+++ b/pkg/controllers/clusterops/controller_test.go
@@ -89,8 +89,8 @@ func TestUpdateStatusLoop(t *testing.T) {
 			name: "the status is Running and the result of fetchJobStatus is err",
 			args: func(ops *clusteroperationv1alpha1.ClusterOperation) bool {
 				ops.Status.Status = clusteroperationv1alpha1.RunningStatus
-				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, error) {
-					return "", fmt.Errorf("one error")
+				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, *metav1.Time, error) {
+					return "", nil, fmt.Errorf("one error")
 				})
 				return err != nil && err.Error() == "one error" && !needRequeue
 			},
@@ -100,8 +100,8 @@ func TestUpdateStatusLoop(t *testing.T) {
 			name: "the status is Running and the result of fetchJobStatus is still Running",
 			args: func(ops *clusteroperationv1alpha1.ClusterOperation) bool {
 				ops.Status.Status = clusteroperationv1alpha1.RunningStatus
-				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, error) {
-					return clusteroperationv1alpha1.RunningStatus, nil
+				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, *metav1.Time, error) {
+					return clusteroperationv1alpha1.RunningStatus, nil, nil
 				})
 				return err == nil && needRequeue
 			},
@@ -112,8 +112,8 @@ func TestUpdateStatusLoop(t *testing.T) {
 			args: func(ops *clusteroperationv1alpha1.ClusterOperation) bool {
 				ops.Status.Status = clusteroperationv1alpha1.RunningStatus
 				ops.Status.EndTime = nil
-				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, error) {
-					return clusteroperationv1alpha1.SucceededStatus, nil
+				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, *metav1.Time, error) {
+					return clusteroperationv1alpha1.SucceededStatus, &metav1.Time{}, nil
 				})
 				resultOps := &clusteroperationv1alpha1.ClusterOperation{}
 				controller.Client.Get(context.Background(), client.ObjectKey{Name: "clusteropsname"}, resultOps)
@@ -130,8 +130,8 @@ func TestUpdateStatusLoop(t *testing.T) {
 				controller.Client.Get(context.Background(), client.ObjectKey{Name: "clusteropsname"}, ops)
 				ops.Status.Status = clusteroperationv1alpha1.RunningStatus
 				ops.Status.EndTime = nil
-				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, error) {
-					return clusteroperationv1alpha1.FailedStatus, nil
+				needRequeue, err := controller.UpdateStatusLoop(ops, func(ops *clusteroperationv1alpha1.ClusterOperation) (clusteroperationv1alpha1.OpsStatus, *metav1.Time, error) {
+					return clusteroperationv1alpha1.FailedStatus, nil, nil
 				})
 				return err == nil && !needRequeue && ops.Status.EndTime != nil && ops.Status.Status == clusteroperationv1alpha1.FailedStatus
 			},
@@ -1745,7 +1745,7 @@ func Test_FetchJobStatus(t *testing.T) {
 			name: "jobRef is empty",
 			args: func() bool {
 				clusterOps := &clusteroperationv1alpha1.ClusterOperation{}
-				_, err := controller.FetchJobStatus(clusterOps)
+				_, _, err := controller.FetchJobConditionStatusAndCompletionTime(clusterOps)
 				return err != nil
 			},
 			want: true,
@@ -1755,7 +1755,7 @@ func Test_FetchJobStatus(t *testing.T) {
 			args: func() bool {
 				clusterOps := &clusteroperationv1alpha1.ClusterOperation{}
 				clusterOps.Status.JobRef = &apis.JobRef{NameSpace: "kubean-system", Name: "job123"}
-				status, err := controller.FetchJobStatus(clusterOps)
+				status, _, err := controller.FetchJobConditionStatusAndCompletionTime(clusterOps)
 				return err == nil && status == clusteroperationv1alpha1.FailedStatus
 			},
 			want: true,
@@ -1784,7 +1784,7 @@ func Test_FetchJobStatus(t *testing.T) {
 					},
 				}
 				controller.ClientSet.BatchV1().Jobs("kubean-system").Create(context.Background(), targetJob, metav1.CreateOptions{})
-				status, err := controller.FetchJobStatus(clusterOps)
+				status, _, err := controller.FetchJobConditionStatusAndCompletionTime(clusterOps)
 				return err == nil && status == clusteroperationv1alpha1.SucceededStatus
 			},
 			want: true,
@@ -1813,7 +1813,7 @@ func Test_FetchJobStatus(t *testing.T) {
 					},
 				}
 				controller.ClientSet.BatchV1().Jobs("kubean-system").Create(context.Background(), targetJob, metav1.CreateOptions{})
-				status, err := controller.FetchJobStatus(clusterOps)
+				status, _, err := controller.FetchJobConditionStatusAndCompletionTime(clusterOps)
 				return err == nil && status == clusteroperationv1alpha1.FailedStatus
 			},
 			want: true,
@@ -1834,8 +1834,8 @@ func Test_FetchJobStatus(t *testing.T) {
 					},
 				}
 				controller.ClientSet.BatchV1().Jobs("kubean-system").Create(context.Background(), targetJob, metav1.CreateOptions{})
-				status, err := controller.FetchJobStatus(clusterOps)
-				return err == nil && status == clusteroperationv1alpha1.RunningStatus
+				status, completionTime, err := controller.FetchJobConditionStatusAndCompletionTime(clusterOps)
+				return err == nil && status == clusteroperationv1alpha1.RunningStatus && completionTime == nil
 			},
 			want: true,
 		},