Include MetricsUnavailable condition to Complete in Trial (#1877)

* include MetricsUnavailable condition to Complete in Trial

It is not easy for users to find why Trial failed when training code output incorrect format logs
since the trial-controller sets Succeeded condition with False to Trial if there are unavailable metrics in Katib DB as described in #1343.
So we also include MetricsUnavailable condition to Complete in Trial.

* add gh-actions tasks to verify generated codes

* fix gh-actions workflow

* when the number of Failed Trials reaches maxTrialCount, experiment-controller sets Failed to Experiment status

* fix e2e test

* To avoid being set Failed in Experiment status when  and  is equal to 0, we need to add condition,

Author: tenzen-y

.github/workflows/test-go.yaml

@@ -5,15 +5,14 @@ on:
   - pull_request
 
 jobs:
-  test:
-    name: Test
+  generatetests:
+    name: Generate And Format Test
     runs-on: ubuntu-latest
     env:
       GOPATH: ${{ github.workspace }}/go
     defaults:
       run:
         working-directory: ${{ env.GOPATH }}/src/github.com/kubeflow/katib
-
     steps:
       - name: Check out code
         uses: actions/checkout@v2
@@ -28,19 +27,47 @@ jobs:
       # Verify that go.mod and go.sum is synchronized
       - name: Check Go modules
         run: |
-          if [[ ! -z $(go mod tidy && git diff --exit-code) ]]; then
-            echo "Please run "go mod tidy" to sync Go modules"
-            exit 1
-          fi
+          go mod tidy &&
+          git add go.* &&
+          git diff --cached --exit-code || (echo 'Please run "go mod tidy" to sync Go modules' && exit 1)
 
-      - name: Run Go test
+      - name: Run Generate And Go Format Test
         run: |
-          go mod download
-          make check
-          make test
+          go mod download &&
+          make check &&
+          git add pkg/apis hack/gen-python-sdk &&
+          git diff --cached --exit-code || (echo 'Please run "make check" to generate codes and to format Go codes' && exit 1)
+
+  unittests:
+    name: Unit Test
+    runs-on: ubuntu-latest
+    env:
+      GOPATH: ${{ github.workspace }}/go
+    defaults:
+      run:
+        working-directory: ${{ env.GOPATH }}/src/github.com/kubeflow/katib
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v2
+        with:
+          path: ${{ env.GOPATH }}/src/github.com/kubeflow/katib
+
+      - name: Setup Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17.10
+
+      - name: Run Go test
+        run: go mod download && make test ENVTEST_K8S_VERSION=${{ matrix.kubernetes-version }}
 
       - name: Coveralls report
         uses: shogo82148/actions-goveralls@v1
         with:
           path-to-profile: coverage.out
           working-directory: ${{ env.GOPATH }}/src/github.com/kubeflow/katib
+
+    strategy:
+      fail-fast: false
+      matrix:
+        # Detail: `setup-envtest list --arch amd64`
+        kubernetes-version: ["1.21.4", "1.22.1", "1.23.5"]

Makefile

@@ -24,7 +24,6 @@ ifndef HAS_SETUP_ENVTEST
 endif
 	@echo "setup-envtest has already installed"
 
-
 check: generate fmt vet lint
 
 fmt:

pkg/apis/controller/experiments/v1beta1/experiment_types.go

@@ -100,6 +100,9 @@ type ExperimentStatus struct {
 	// List of trial names which have been early stopped.
 	EarlyStoppedTrialList []string `json:"earlyStoppedTrialList,omitempty"`
 
+	// List of trial names which have been metrics unavailable
+	MetricsUnavailableTrialList []string `json:"metricsUnavailableTrialList,omitempty"`
+
 	// Trials is the total number of trials owned by the experiment.
 	Trials int32 `json:"trials,omitempty"`
 
@@ -120,6 +123,9 @@ type ExperimentStatus struct {
 
 	// How many trials are currently early stopped.
 	TrialsEarlyStopped int32 `json:"trialsEarlyStopped,omitempty"`
+
+	// How many trials are currently metrics unavailable.
+	TrialMetricsUnavailable int32 `json:"trialMetricsUnavailable,omitempty"`
 }
 
 // OptimalTrial is the metrics and assignments of the best trial.

pkg/apis/controller/experiments/v1beta1/zz_generated.deepcopy.go

@@ -113,12 +113,13 @@ type TrialCondition struct {
 type TrialConditionType string
 
 const (
-	TrialCreated      TrialConditionType = "Created"
-	TrialRunning      TrialConditionType = "Running"
-	TrialSucceeded    TrialConditionType = "Succeeded"
-	TrialKilled       TrialConditionType = "Killed"
-	TrialFailed       TrialConditionType = "Failed"
-	TrialEarlyStopped TrialConditionType = "EarlyStopped"
+	TrialCreated            TrialConditionType = "Created"
+	TrialRunning            TrialConditionType = "Running"
+	TrialSucceeded          TrialConditionType = "Succeeded"
+	TrialKilled             TrialConditionType = "Killed"
+	TrialFailed             TrialConditionType = "Failed"
+	TrialMetricsUnavailable TrialConditionType = "MetricsUnavailable"
+	TrialEarlyStopped       TrialConditionType = "EarlyStopped"
 )
 
 // +genclient

pkg/apis/controller/trials/v1beta1/trial_types.go

@@ -86,15 +86,11 @@ func (trial *Trial) IsKilled() bool {
 
 // IsMetricsUnavailable returns true if Trial metrics are not available
 func (trial *Trial) IsMetricsUnavailable() bool {
-	cond := getCondition(trial, TrialSucceeded)
-	if cond != nil && cond.Status == v1.ConditionFalse {
-		return true
-	}
-	return false
+	return hasCondition(trial, TrialMetricsUnavailable)
 }
 
 func (trial *Trial) IsCompleted() bool {
-	return trial.IsSucceeded() || trial.IsFailed() || trial.IsKilled() || trial.IsEarlyStopped()
+	return trial.IsSucceeded() || trial.IsFailed() || trial.IsKilled() || trial.IsEarlyStopped() || trial.IsMetricsUnavailable()
 }
 
 func (trial *Trial) IsEarlyStopped() bool {
@@ -158,3 +154,11 @@ func (trial *Trial) MarkTrialStatusKilled(reason, message string) {
 	}
 	trial.setCondition(TrialKilled, v1.ConditionTrue, reason, message)
 }
+
+func (trial *Trial) MarkTrialStatusMetricsUnavailable(reason, message string) {
+	currentCond := getCondition(trial, TrialRunning)
+	if currentCond != nil {
+		trial.setCondition(TrialRunning, v1.ConditionFalse, currentCond.Reason, currentCond.Message)
+	}
+	trial.setCondition(TrialMetricsUnavailable, v1.ConditionTrue, reason, message)
+}