Merge pull request #116 from whywaita/fix/114

Use same delete logic in mode once as a mode epehemeral
whywaita · Nov 1, 2021 · 1212edd · 1212edd
2 parents bc506fd + 8c920d4
commit 1212edd
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 13 deletions.
diff --git a/pkg/datastore/interface.go b/pkg/datastore/interface.go
@@ -48,6 +48,7 @@ type Datastore interface {
 
 	CreateRunner(ctx context.Context, runner Runner) error
 	ListRunners(ctx context.Context) ([]Runner, error)
+	ListRunnersByTargetID(ctx context.Context, targetID uuid.UUID) ([]Runner, error)
 	GetRunner(ctx context.Context, id uuid.UUID) (*Runner, error)
 	DeleteRunner(ctx context.Context, id uuid.UUID, deletedAt time.Time, reason RunnerStatus) error
 

diff --git a/pkg/datastore/memory/memory.go b/pkg/datastore/memory/memory.go
@@ -223,6 +223,21 @@ func (m *Memory) ListRunners(ctx context.Context) ([]datastore.Runner, error) {
 	return runners, nil
 }
 
+// ListRunnersByTargetID get a not deleted runners that has target_id
+func (m *Memory) ListRunnersByTargetID(ctx context.Context, targetID uuid.UUID) ([]datastore.Runner, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	var runners []datastore.Runner
+	for _, r := range m.runners {
+		if uuid.Equal(r.TargetID, targetID) {
+			runners = append(runners, r)
+		}
+	}
+
+	return runners, nil
+}
+
 // GetRunner get a runner
 func (m *Memory) GetRunner(ctx context.Context, id uuid.UUID) (*datastore.Runner, error) {
 	m.mu.Lock()

diff --git a/pkg/datastore/mysql/runner.go b/pkg/datastore/mysql/runner.go
@@ -57,6 +57,23 @@ func (m *MySQL) ListRunners(ctx context.Context) ([]datastore.Runner, error) {
 	return runners, nil
 }
 
+// ListRunnersByTargetID get a not deleted runners that has target_id
+func (m *MySQL) ListRunnersByTargetID(ctx context.Context, targetID uuid.UUID) ([]datastore.Runner, error) {
+	var runners []datastore.Runner
+	query := `SELECT runner.runner_id, detail.shoes_type, detail.ip_address, detail.target_id, detail.cloud_id, detail.created_at, detail.updated_at, detail.resource_type, detail.repository_url, detail.request_webhook, detail.runner_user, detail.runner_version, detail.provider_url
+ FROM runners_running AS runner JOIN runner_detail AS detail ON runner.runner_id = detail.runner_id WHERE detail.target_id = ?`
+	err := m.Conn.SelectContext(ctx, &runners, query, targetID)
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return nil, datastore.ErrNotFound
+		}
+
+		return nil, fmt.Errorf("failed to execute SELECT query: %w", err)
+	}
+
+	return runners, nil
+}
+
 // GetRunner get a runner
 func (m *MySQL) GetRunner(ctx context.Context, id uuid.UUID) (*datastore.Runner, error) {
 	var r datastore.Runner

diff --git a/pkg/runner/runner_delete.go b/pkg/runner/runner_delete.go
@@ -31,16 +31,16 @@ func (m *Manager) do(ctx context.Context) error {
 	logger.Logf(true, "found %d targets in datastore", len(targets))
 	for _, target := range targets {
 		logger.Logf(true, "start to search runner in %s", target.RepoURL())
-		if err := m.removeRunners(ctx, &target); err != nil {
+		if err := m.removeRunners(ctx, target); err != nil {
 			logger.Logf(false, "failed to delete runners (target: %s): %+v", target.RepoURL(), err)
 		}
 	}
 
 	return nil
 }
 
-func (m *Manager) removeRunners(ctx context.Context, t *datastore.Target) error {
-	runners, err := m.ds.ListRunners(ctx)
+func (m *Manager) removeRunners(ctx context.Context, t datastore.Target) error {
+	runners, err := m.ds.ListRunnersByTargetID(ctx, t.UUID)
 	if err != nil {
 		return fmt.Errorf("failed to retrieve list of running runner: %w", err)
 	}
@@ -78,7 +78,7 @@ func (m *Manager) removeRunners(ctx context.Context, t *datastore.Target) error
 	return nil
 }
 
-func isRegisteredRunnerZeroInGitHub(ctx context.Context, t *datastore.Target) (bool, error) {
+func isRegisteredRunnerZeroInGitHub(ctx context.Context, t datastore.Target) (bool, error) {
 	owner, repo := t.OwnerRepo()
 	client, err := gh.NewClient(ctx, t.GitHubToken, t.GHEDomain.String)
 	if err != nil {

diff --git a/pkg/runner/runner_delete_ephemeral.go b/pkg/runner/runner_delete_ephemeral.go
@@ -12,7 +12,7 @@ import (
 
 // removeRunnerModeEphemeral remove runner that created by --ephemeral flag.
 // --ephemeral flag is delete self-hosted runner when end of job. So, The origin list of runner from datastore.
-func (m *Manager) removeRunnerModeEphemeral(ctx context.Context, t *datastore.Target, runner datastore.Runner) error {
+func (m *Manager) removeRunnerModeEphemeral(ctx context.Context, t datastore.Target, runner datastore.Runner) error {
 	owner, repo := t.OwnerRepo()
 	client, err := gh.NewClient(ctx, t.GitHubToken, t.GHEDomain.String)
 	if err != nil {

diff --git a/pkg/runner/runner_delete_once.go b/pkg/runner/runner_delete_once.go
@@ -12,16 +12,27 @@ import (
 
 // removeRunnerModeOnce remove runner that created by --once flag.
 // --once flag is not delete self-hosted runner when end of job. So, The origin list of runner from GitHub.
-func (m *Manager) removeRunnerModeOnce(ctx context.Context, t *datastore.Target, runner datastore.Runner) error {
+func (m *Manager) removeRunnerModeOnce(ctx context.Context, t datastore.Target, runner datastore.Runner) error {
 	owner, repo := t.OwnerRepo()
 	client, err := gh.NewClient(ctx, t.GitHubToken, t.GHEDomain.String)
 	if err != nil {
 		return fmt.Errorf("failed to create github client: %w", err)
 	}
 
 	ghRunner, err := gh.ExistGitHubRunner(ctx, client, owner, repo, ToName(runner.UUID.String()))
-	if err != nil {
-		return fmt.Errorf("failed to get runner from GitHub (runner: %s): %w", runner.UUID.String(), err)
+	switch {
+	case errors.Is(err, gh.ErrNotFound):
+		logger.Logf(false, "NotFound in GitHub, so will delete in datastore without GitHub (runner: %s)", runner.UUID.String())
+		if err := m.deleteRunner(ctx, runner, StatusWillDelete); err != nil {
+			if err := datastore.UpdateTargetStatus(ctx, m.ds, t.UUID, datastore.TargetStatusErr, ""); err != nil {
+				logger.Logf(false, "failed to update target status (target ID: %s): %+v\n", t.UUID, err)
+			}
+
+			return fmt.Errorf("failed to delete runner: %w", err)
+		}
+		return nil
+	case err != nil:
+		return fmt.Errorf("failed to check runner exist in GitHub (runner: %s): %w", runner.UUID, err)
 	}
 
 	if err := sanitizeGitHubRunner(*ghRunner, runner); err != nil {

diff --git a/pkg/web/target.go b/pkg/web/target.go
@@ -140,7 +140,7 @@ func handleTargetList(w http.ResponseWriter, r *http.Request, ds datastore.Datas
 
 	var targets []UserTarget
 	for _, t := range ts {
-		ut := sanitizeTarget(&t)
+		ut := sanitizeTarget(t)
 		targets = append(targets, ut)
 	}
 
@@ -168,15 +168,15 @@ func handleTargetRead(w http.ResponseWriter, r *http.Request, ds datastore.Datas
 		return
 	}
 
-	ut := sanitizeTarget(target)
+	ut := sanitizeTarget(*target)
 
 	w.Header().Set("Content-Type", "application/json;charset=utf-8")
 	w.WriteHeader(http.StatusOK)
 	json.NewEncoder(w).Encode(ut)
 	return
 }
 
-func sanitizeTarget(t *datastore.Target) UserTarget {
+func sanitizeTarget(t datastore.Target) UserTarget {
 	ut := UserTarget{
 		UUID:              t.UUID,
 		Scope:             t.Scope,
@@ -247,7 +247,7 @@ func handleTargetUpdate(w http.ResponseWriter, r *http.Request, ds datastore.Dat
 		outputErrorMsg(w, http.StatusInternalServerError, "datastore get error")
 		return
 	}
-	ut := sanitizeTarget(updatedTarget)
+	ut := sanitizeTarget(*updatedTarget)
 
 	w.Header().Set("Content-Type", "application/json;charset=utf-8")
 	w.WriteHeader(http.StatusOK)

diff --git a/pkg/web/target_create.go b/pkg/web/target_create.go
@@ -112,7 +112,7 @@ func handleTargetCreate(w http.ResponseWriter, r *http.Request, ds datastore.Dat
 		outputErrorMsg(w, http.StatusInternalServerError, "datastore get error")
 		return
 	}
-	ut := sanitizeTarget(createdTarget)
+	ut := sanitizeTarget(*createdTarget)
 
 	w.Header().Set("Content-Type", "application/json;charset=utf-8")
 	w.WriteHeader(http.StatusCreated)