Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add test for pipedv1's buildPlan method, and remove platform specific config from configv1's GenericApplicationConfig #5238

Merged
merged 9 commits into from
Oct 1, 2024
Merged

Add test for pipedv1's buildPlan method, and remove platform specific config from configv1's GenericApplicationConfig #5238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
25462da
Add test for buildPlan
Warashi Sep 26, 2024
b682e5e
Fix test input
Warashi Sep 27, 2024
4807633
Remove platform-specific fields from GenericApplicationSpec
Warashi Sep 27, 2024
48aa3f8
Remove broken k8s-specific test from configv1
Warashi Sep 27, 2024
921e4d0
Fix the test for pipedv1 buildPlan
Warashi Sep 27, 2024
7195cc8
Remove mistakenly commited changes
Warashi Sep 27, 2024
d9d098c
Remove stage-specific tests that are broken
Warashi Sep 27, 2024
b9e9261
Move parsing GenericApplicationSpec to configv1
Warashi Oct 1, 2024
85c0a79
Add KindApplication and remove ParseApplication
Warashi Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 45 additions & 38 deletions pkg/app/pipedv1/controller/planner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
"context"
"encoding/json"
"fmt"
"io"
"path/filepath"
"sort"
"time"

Expand All @@ -30,10 +28,9 @@
"go.uber.org/zap"

"github.com/pipe-cd/pipecd/pkg/app/pipedv1/controller/controllermetrics"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/deploysource"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/metadatastore"
"github.com/pipe-cd/pipecd/pkg/app/server/service/pipedservice"
"github.com/pipe-cd/pipecd/pkg/config"
"github.com/pipe-cd/pipecd/pkg/configv1"
"github.com/pipe-cd/pipecd/pkg/model"
pluginapi "github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1"
"github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1/deployment"
Expand Down Expand Up @@ -192,27 +189,30 @@
controllermetrics.UpdateDeploymentStatus(p.deployment, p.doneDeploymentStatus)
}()

repoCfg := config.PipedRepository{
RepoID: p.deployment.GitPath.Repo.Id,
Remote: p.deployment.GitPath.Repo.Remote,
Branch: p.deployment.GitPath.Repo.Branch,
}
// TODO: Prepare running deploy source and target deploy source.
var runningDS, targetDS *model.DeploymentSource

Check warning on line 193 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L193 

Added line #L193 was not covered by tests

// Prepare target deploy source.
targetDSP := deploysource.NewProvider(
filepath.Join(p.workingDir, "deploysource"),
deploysource.NewGitSourceCloner(p.gitClient, repoCfg, "target", p.deployment.Trigger.Commit.Hash),
*p.deployment.GitPath,
nil, // TODO: Revise this secret decryter, is this need?
)
// repoCfg := config.PipedRepository{
// RepoID: p.deployment.GitPath.Repo.Id,
// Remote: p.deployment.GitPath.Repo.Remote,
// Branch: p.deployment.GitPath.Repo.Branch,
// }

Check warning on line 199 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L195-L199 

Added lines #L195 - L199 were not covered by tests

targetDS, err := targetDSP.Get(ctx, io.Discard)
if err != nil {
return fmt.Errorf("error while preparing deploy source data (%v)", err)
}
// Prepare target deploy source.
// targetDSP := deploysource.NewProvider(
// filepath.Join(p.workingDir, "deploysource"),
// deploysource.NewGitSourceCloner(p.gitClient, repoCfg, "target", p.deployment.Trigger.Commit.Hash),
// *p.deployment.GitPath,
// nil, // TODO: Revise this secret decryter, is this need?
// )

// targetDS, err := targetDSP.Get(ctx, io.Discard)
// if err != nil {
// return fmt.Errorf("error while preparing deploy source data (%v)", err)
// }

Check warning on line 212 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L201-L212 

Added lines #L201 - L212 were not covered by tests

// TODO: Pass running DS as well if need?
out, err := p.buildPlan(ctx, targetDS)
out, err := p.buildPlan(ctx, runningDS, targetDS)

Check warning on line 215 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L215 

Added line #L215 was not covered by tests

// If the deployment was already cancelled, we ignore the plan result.
select {
Expand Down Expand Up @@ -243,13 +243,15 @@
// - CommitMatcher ensure pipeline/quick sync based on the commit message
// - Force quick sync if there is no previous deployment (aka. this is the first deploy)
// - Based on PlannerService.DetermineStrategy returned by plugins
func (p *planner) buildPlan(ctx context.Context, targetDS *deploysource.DeploySource) (*plannerOutput, error) {
func (p *planner) buildPlan(ctx context.Context, runningDS, targetDS *model.DeploymentSource) (*plannerOutput, error) {
out := &plannerOutput{}

input := &deployment.PlanPluginInput{
Deployment: p.deployment,
Deployment: p.deployment,
RunningDeploymentSource: runningDS,
TargetDeploymentSource: targetDS,
// TODO: Add more planner input fields.
// NOTE: As discussed we pass targetDS & runningDS here.
// we need passing PluginConfig
}

// Build deployment target versions.
Expand All @@ -270,20 +272,25 @@
}
}

cfg := targetDS.GenericApplicationConfig
cfg, err := config.DecodeYAML(targetDS.GetApplicationConfig())
if err != nil {
p.logger.Error("unable to parse application config", zap.Error(err))
return nil, err
}

Check warning on line 279 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L277-L279 

Added lines #L277 - L279 were not covered by tests
spec := cfg.ApplicationSpec

// In case the strategy has been decided by trigger.
// For example: user triggered the deployment via web console.
switch p.deployment.Trigger.SyncStrategy {
case model.SyncStrategy_QUICK_SYNC:
if stages, err := p.buildQuickSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildQuickSyncStages(ctx, spec); err == nil {
out.SyncStrategy = model.SyncStrategy_QUICK_SYNC
out.Summary = p.deployment.Trigger.StrategySummary
out.Stages = stages
return out, nil
}
case model.SyncStrategy_PIPELINE:
if stages, err := p.buildPipelineSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildPipelineSyncStages(ctx, spec); err == nil {
out.SyncStrategy = model.SyncStrategy_PIPELINE
out.Summary = p.deployment.Trigger.StrategySummary
out.Stages = stages
Expand All @@ -292,8 +299,8 @@
}

// When no pipeline was configured, do the quick sync.
if cfg.Pipeline == nil || len(cfg.Pipeline.Stages) == 0 {
if stages, err := p.buildQuickSyncStages(ctx, cfg); err == nil {
if spec.Pipeline == nil || len(spec.Pipeline.Stages) == 0 {
if stages, err := p.buildQuickSyncStages(ctx, spec); err == nil {
out.SyncStrategy = model.SyncStrategy_QUICK_SYNC
out.Summary = "Quick sync due to the pipeline was not configured"
out.Stages = stages
Expand All @@ -302,8 +309,8 @@
}

// Force to use pipeline when the `spec.planner.alwaysUsePipeline` was configured.
if cfg.Planner.AlwaysUsePipeline {
if stages, err := p.buildPipelineSyncStages(ctx, cfg); err == nil {
if spec.Planner.AlwaysUsePipeline {
if stages, err := p.buildPipelineSyncStages(ctx, spec); err == nil {
out.SyncStrategy = model.SyncStrategy_PIPELINE
out.Summary = "Sync with the specified pipeline (alwaysUsePipeline was set)"
out.Stages = stages
Expand All @@ -315,10 +322,10 @@

// This deployment is triggered by a commit with the intent to perform pipeline.
// Commit Matcher will be ignored when triggered by a command.
if pattern := cfg.CommitMatcher.Pipeline; pattern != "" && p.deployment.Trigger.Commander == "" {
if pattern := spec.CommitMatcher.Pipeline; pattern != "" && p.deployment.Trigger.Commander == "" {
if pipelineRegex, err := regexPool.Get(pattern); err == nil &&
pipelineRegex.MatchString(p.deployment.Trigger.Commit.Message) {
if stages, err := p.buildPipelineSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildPipelineSyncStages(ctx, spec); err == nil {

Check warning on line 328 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L328 

Added line #L328 was not covered by tests
out.SyncStrategy = model.SyncStrategy_PIPELINE
out.Summary = fmt.Sprintf("Sync progressively because the commit message was matching %q", pattern)
out.Stages = stages
Expand All @@ -329,10 +336,10 @@

// This deployment is triggered by a commit with the intent to synchronize.
// Commit Matcher will be ignored when triggered by a command.
if pattern := cfg.CommitMatcher.QuickSync; pattern != "" && p.deployment.Trigger.Commander == "" {
if pattern := spec.CommitMatcher.QuickSync; pattern != "" && p.deployment.Trigger.Commander == "" {
if syncRegex, err := regexPool.Get(pattern); err == nil &&
syncRegex.MatchString(p.deployment.Trigger.Commit.Message) {
if stages, err := p.buildQuickSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildQuickSyncStages(ctx, spec); err == nil {

Check warning on line 342 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L342 

Added line #L342 was not covered by tests
out.SyncStrategy = model.SyncStrategy_QUICK_SYNC
out.Summary = fmt.Sprintf("Quick sync because the commit message was matching %q", pattern)
out.Stages = stages
Expand All @@ -343,7 +350,7 @@

// Quick sync if this is the first time to deploy this application or it was unable to retrieve running commit hash.
if p.lastSuccessfulCommitHash == "" {
if stages, err := p.buildQuickSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildQuickSyncStages(ctx, spec); err == nil {
out.SyncStrategy = model.SyncStrategy_QUICK_SYNC
out.Summary = "Quick sync, it seems this is the first deployment of the application"
out.Stages = stages
Expand Down Expand Up @@ -372,14 +379,14 @@

switch strategy {
case model.SyncStrategy_QUICK_SYNC:
if stages, err := p.buildQuickSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildQuickSyncStages(ctx, spec); err == nil {

Check warning on line 382 in pkg/app/pipedv1/controller/planner.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/controller/planner.go#L382 

Added line #L382 was not covered by tests
out.SyncStrategy = model.SyncStrategy_QUICK_SYNC
out.Summary = summary
out.Stages = stages
return out, nil
}
case model.SyncStrategy_PIPELINE:
if stages, err := p.buildPipelineSyncStages(ctx, cfg); err == nil {
if stages, err := p.buildPipelineSyncStages(ctx, spec); err == nil {
out.SyncStrategy = model.SyncStrategy_PIPELINE
out.Summary = summary
out.Stages = stages
Expand Down
Loading
Loading