Implement live state reporter for Cloud Run #3284

knanao · 2022-02-17T15:23:10Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Implement live state reporter for Cloud Run

pipecd-bot · 2022-02-17T15:24:00Z

The following files are not gofmt-ed. By commenting /golinter fmt, the formatted one will be appended to this pull request automatically.

pkg/app/piped/livestatereporter/cloudrun/report.go

--- pkg/app/piped/livestatereporter/cloudrun/report.go.orig
+++ pkg/app/piped/livestatereporter/cloudrun/report.go
@@ -19,12 +19,13 @@
 	"fmt"
 	"time"
 
+	"go.uber.org/zap"
+	"google.golang.org/grpc"
+
 	"github.com/pipe-cd/pipecd/pkg/app/piped/livestatestore/cloudrun"
 	"github.com/pipe-cd/pipecd/pkg/app/server/service/pipedservice"
 	"github.com/pipe-cd/pipecd/pkg/config"
 	"github.com/pipe-cd/pipecd/pkg/model"
-	"go.uber.org/zap"
-	"google.golang.org/grpc"
 )
 
 type applicationLister interface {

knanao · 2022-02-17T15:24:28Z

/golinter fmt

pipecd-bot · 2022-02-17T15:42:00Z

Code coverage for golang is 32.33%. This pull request increases coverage by 0.15%.

File	Function	Base	Head	Diff
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.determineKubernetesAppHealthStatus	`--`	`100.00%`	`+100.00%`
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.determineCloudRunAppHealthStatus	`--`	`100.00%`	`+100.00%`
pkg/app/pipectl/cmd/planpreview/planpreview.go	ReadableResult.String	`96.67%`	`94.44%`	`-2.22%`
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.DetermineAppHealthStatus	`0.00%`	`100.00%`	`+100.00%`
pkg/model/planpreview.go	PlanPreviewCommandResult.FillURLs	`0.00%`	`0.00%`	`+0.00%`

pkg/app/piped/livestatereporter/cloudrun/report.go

pipecd-bot · 2022-02-20T01:52:45Z

Code coverage for golang is 32.33%. This pull request increases coverage by 0.15%.

File	Function	Base	Head	Diff
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.determineKubernetesAppHealthStatus	`--`	`100.00%`	`+100.00%`
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.determineCloudRunAppHealthStatus	`--`	`100.00%`	`+100.00%`
pkg/app/pipectl/cmd/planpreview/planpreview.go	ReadableResult.String	`96.67%`	`94.44%`	`-2.22%`
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.DetermineAppHealthStatus	`0.00%`	`100.00%`	`+100.00%`
pkg/model/planpreview.go	PlanPreviewCommandResult.FillURLs	`0.00%`	`0.00%`	`+0.00%`

knanao · 2022-02-21T04:35:26Z

hold this PR until v0.26.0 released
/hold

knanao · 2022-02-21T06:11:44Z

/hold cancel

nghialv · 2022-02-21T06:13:14Z

pkg/app/piped/livestatereporter/cloudrun/report.go

+
+	for {
+		select {
+		case <-snapshotTicker.C:


This means that the state will be updated every 10 minutes, right?
Do we have any way to make it refresh faster on UI?

@nghialv

This means that the state will be updated every 10 minutes, right?

Exactly.

Do we have any way to make it refresh faster on UI?

Currently, No we don't. Since livestatestore fetch the live state at 15s intervals and driftdetection and livestatereporter refer to it. But as one idea, it may be better that adding forceRefresh flag into GetApplicationLiveStateRequest in order to get latest live state synchronously.
BTW, It might be a good idea to make this interval one minute like as driftdetection.

pipecd-bot · 2022-02-21T07:35:00Z

Code coverage for golang is 32.33%. This pull request increases coverage by 0.15%.

File	Function	Base	Head	Diff
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.determineKubernetesAppHealthStatus	`--`	`100.00%`	`+100.00%`
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.determineCloudRunAppHealthStatus	`--`	`100.00%`	`+100.00%`
pkg/app/pipectl/cmd/planpreview/planpreview.go	ReadableResult.String	`96.67%`	`94.44%`	`-2.22%`
pkg/model/application_live_state.go	ApplicationLiveStateSnapshot.DetermineAppHealthStatus	`0.00%`	`100.00%`	`+100.00%`
pkg/model/planpreview.go	PlanPreviewCommandResult.FillURLs	`0.00%`	`0.00%`	`+0.00%`

nghialv · 2022-02-21T07:54:22Z

Nice.
/lgtm

nghialv · 2022-02-21T07:55:01Z

pkg/app/piped/livestatereporter/cloudrun/report.go

+			Snapshot: snapshot,
+		}
+
+		if _, err := r.apiClient.ReportApplicationLiveState(ctx, req); err != nil {


nit: We may need a new RPC to send a bunch of applications.

Yep, I think it's good to add new RPC to improve performance in the future.

khanhtc1202 · 2022-02-21T09:01:39Z

Here you go 🙌
/approve

pipecd-bot · 2022-02-21T09:01:42Z

This pull request is APPROVED by khanhtc1202.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

pipecd-bot added the area/piped label Feb 17, 2022

pipecd-bot requested review from khanhtc1202 and nghialv February 17, 2022 15:23

pipecd-bot added the size/L label Feb 17, 2022

Implement live state reporter for Cloud Run

c0446e1

knanao force-pushed the livestatereporter branch from 75af0c5 to c0446e1 Compare February 17, 2022 15:35

nghialv reviewed Feb 18, 2022

View reviewed changes

pkg/app/piped/livestatereporter/cloudrun/report.go Outdated Show resolved Hide resolved

Remove env id from snapshot

d817d75

pipecd-bot added the do-not-merge/hold label Feb 21, 2022

pipecd-bot removed the do-not-merge/hold label Feb 21, 2022

nghialv reviewed Feb 21, 2022

View reviewed changes

Fix flush interval

791364e

pipecd-bot added the lgtm label Feb 21, 2022

nghialv reviewed Feb 21, 2022

View reviewed changes

pipecd-bot added the approved label Feb 21, 2022

pipecd-bot merged commit d8d4ad2 into master Feb 21, 2022

pipecd-bot deleted the livestatereporter branch February 21, 2022 09:10

pipecd-bot mentioned this pull request Mar 1, 2022

Remove environment model #3324

Merged

pipecd-bot mentioned this pull request Mar 8, 2022

Move launcher config to config package #3362

Merged

pipecd-bot mentioned this pull request Mar 17, 2022

Fix bug that the specified kubeConfigPath in Piped config was not used while handling manifests #3410

Merged

This was referenced Mar 18, 2022

Use the latest codegen #3414

Merged

Delete unneeded CODEOWNERS file #3416

Merged

Implement live state reporter for Cloud Run #3284

Implement live state reporter for Cloud Run #3284

Uh oh!

Conversation

knanao commented Feb 17, 2022

Uh oh!

pipecd-bot commented Feb 17, 2022

Uh oh!

knanao commented Feb 17, 2022

Uh oh!

pipecd-bot commented Feb 17, 2022

Uh oh!

Uh oh!

pipecd-bot commented Feb 20, 2022

Uh oh!

knanao commented Feb 21, 2022

Uh oh!

knanao commented Feb 21, 2022

Uh oh!

nghialv Feb 21, 2022

Choose a reason for hiding this comment

Uh oh!

knanao Feb 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pipecd-bot commented Feb 21, 2022

Uh oh!

nghialv commented Feb 21, 2022

Uh oh!

nghialv Feb 21, 2022

Choose a reason for hiding this comment

Uh oh!

knanao Feb 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 commented Feb 21, 2022

Uh oh!

pipecd-bot commented Feb 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

knanao Feb 21, 2022 •

edited

Loading

knanao Feb 21, 2022 •

edited

Loading