Skip to content

Implement FetchDefinedStages and BuildQuickSyncStages with plugin SDK#5605

Merged
ffjlabo merged 3 commits intomasterfrom
impplement-mothods-to-execute-plan-for-k8s-quicksync
Feb 27, 2025
Merged

Implement FetchDefinedStages and BuildQuickSyncStages with plugin SDK#5605
ffjlabo merged 3 commits intomasterfrom
impplement-mothods-to-execute-plan-for-k8s-quicksync

Conversation

@ffjlabo
Copy link
Member

@ffjlabo ffjlabo commented Feb 26, 2025

What this PR does:

as title.
Also, I checked the plan is working with the patch below.

patch
diff --git a/pkg/app/pipedv1/plugin/kubernetes/main.go b/pkg/app/pipedv1/plugin/kubernetes/main.go
index 0b2450f4a..c8c7ad604 100644
--- a/pkg/app/pipedv1/plugin/kubernetes/main.go
+++ b/pkg/app/pipedv1/plugin/kubernetes/main.go
@@ -18,25 +18,24 @@ import (
        "log"

        kubeconfig "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config"
-       "github.com/pipe-cd/pipecd/pkg/cli"
        "github.com/pipe-cd/pipecd/pkg/plugin/sdk"
 )

-func main() {
-       app := cli.NewApp(
-               "pipecd-plugin-kubernetes",
-               "Plugin component to deploy Kubernetes Application.",
-       )
-       app.AddCommands(
-               NewPluginCommand(),
-       )
-       if err := app.Run(); err != nil {
-               log.Fatal(err)
-       }
-}
+// func main() {
+//     app := cli.NewApp(
+//             "pipecd-plugin-kubernetes",
+//             "Plugin component to deploy Kubernetes Application.",
+//     )
+//     app.AddCommands(
+//             NewPluginCommand(),
+//     )
+//     if err := app.Run(); err != nil {
+//             log.Fatal(err)
+//     }
+// }

 // TODO: use this after rewriting the plugin with the sdk
-func _main() {
+func main() {
        sdk.RegisterDeploymentPlugin[sdk.ConfigNone, kubeconfig.KubernetesDeployTargetConfig](&sdkPlugin{})
        if err := sdk.Run(); err != nil {
                log.Fatalln(err)
diff --git a/pkg/plugin/sdk/deployment.go b/pkg/plugin/sdk/deployment.go
index 40ab7faba..33dbef0f5 100644
--- a/pkg/plugin/sdk/deployment.go
+++ b/pkg/plugin/sdk/deployment.go
@@ -156,10 +156,14 @@ func (s *DeploymentPluginServiceServer[Config, DeployTargetConfig]) FetchDefined
        return &deployment.FetchDefinedStagesResponse{Stages: s.base.FetchDefinedStages()}, nil
 }
 func (s *DeploymentPluginServiceServer[Config, DeployTargetConfig]) DetermineVersions(context.Context, *deployment.DetermineVersionsRequest) (*deployment.DetermineVersionsResponse, error) {
-       return nil, status.Errorf(codes.Unimplemented, "method DetermineVersions not implemented")
+       return &deployment.DetermineVersionsResponse{}, nil
+       // return nil, status.Errorf(codes.Unimplemented, "method DetermineVersions not implemented")
 }
 func (s *DeploymentPluginServiceServer[Config, DeployTargetConfig]) DetermineStrategy(context.Context, *deployment.DetermineStrategyRequest) (*deployment.DetermineStrategyResponse, error) {
-       return nil, status.Errorf(codes.Unimplemented, "method DetermineStrategy not implemented")
+       return &deployment.DetermineStrategyResponse{
+               SyncStrategy: model.SyncStrategy_QUICK_SYNC,
+       }, nil
+       // return nil, status.Errorf(codes.Unimplemented, "method DetermineStrategy not implemented")
 }
 func (s *DeploymentPluginServiceServer[Config, DeployTargetConfig]) BuildPipelineSyncStages(ctx context.Context, request *deployment.BuildPipelineSyncStagesRequest) (*deployment.BuildPipelineSyncStagesResponse, error) {
        client := &Client{

Why we need it:

I want to rewrite some methods to execute the plan.

Which issue(s) this PR fixes:

Part of #4980 #5006

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

@codecov
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 26.56%. Comparing base (e49c4fc) to head (4c2914f).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/app/pipedv1/plugin/kubernetes/plugin.go 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5605      +/-   ##
==========================================
+ Coverage   26.53%   26.56%   +0.03%     
==========================================
  Files         474      474              
  Lines       50505    50535      +30     
==========================================
+ Hits        13399    13427      +28     
- Misses      36042    36045       +3     
+ Partials     1064     1063       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Warashi Warashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost LGTM. I commented on nitpicks.

return out
}

func BuildQuickSyncPipeline(autoRollback bool, now time.Time) []sdk.QuickSyncStage {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 The parameter now seems to be not used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, fixed on e96d964

@ffjlabo ffjlabo requested a review from Warashi February 26, 2025 06:23
@ffjlabo ffjlabo force-pushed the impplement-mothods-to-execute-plan-for-k8s-quicksync branch from e96d964 to c72859b Compare February 26, 2025 06:25
@ffjlabo ffjlabo enabled auto-merge (squash) February 26, 2025 06:39
Copy link
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The PR title would be 'Implement FetchDefinedStages......?' "Implement FetchDefinedStages......"?
because it seems the first implementation rather than rewriting.

@ffjlabo ffjlabo changed the title Rewrite FetchDefinedStages and BuildQuickSyncStages with plugin SDK Implement FetchDefinedStages and BuildQuickSyncStages with plugin SDK Feb 27, 2025
@ffjlabo
Copy link
Member Author

ffjlabo commented Feb 27, 2025

@t-kikuc OK, changed PR title

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo ffjlabo force-pushed the impplement-mothods-to-execute-plan-for-k8s-quicksync branch from c72859b to 4c2914f Compare February 27, 2025 01:02
Copy link
Member

@Warashi Warashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ffjlabo ffjlabo merged commit 9d89463 into master Feb 27, 2025
18 checks passed
@ffjlabo ffjlabo deleted the impplement-mothods-to-execute-plan-for-k8s-quicksync branch February 27, 2025 01:48
@github-actions github-actions bot mentioned this pull request Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants