Skip to content

[SDK] Add type parameter for DeploymentSource and unmarshalling the spec #5740

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

Merged
merged 35 commits into from
Apr 17, 2025
+568 −561
Merged

[SDK] Add type parameter for DeploymentSource and unmarshalling the spec #5740

Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e5dda81
Refactor import statement in config.go for improved readability
Warashi Apr 16, 2025
f9e47e0
Refactor the deployment source into a separate file.
Warashi Apr 16, 2025
629d27e
Add ApplicationConfig type to make it easier to handle application co…
Warashi Apr 16, 2025
e1c797b
Introduce type parameters to DeploymentSource.
Warashi Apr 16, 2025
77fb2c1
Refactor to use type parameters for application config spec.
Warashi Apr 16, 2025
9b2aef2
Add type parameters to the DeploymentPluginServiceServer and StagePlu…
Warashi Apr 16, 2025
3dd620a
Add type parameters to DeploymentPlugin
Warashi Apr 16, 2025
7624171
Fix the type parameters of the plugin.
Warashi Apr 16, 2025
f2e80ad
Fix the type parameters of the LivestatePlugin.
Warashi Apr 16, 2025
419eab1
Fix the type of the livestate plugin.
Warashi Apr 16, 2025
afc87ac
Fix the type errors in the test files
Warashi Apr 16, 2025
e9fbf82
Fix the type of the application config spec in the example plugin.
Warashi Apr 16, 2025
900f69f
Fix the type errors in the Kubernetes deployment plugin.
Warashi Apr 16, 2025
1e28d94
Fix type parameters for the application config
Warashi Apr 16, 2025
03cecc8
Add DecodeYAML method to the deployment source
Warashi Apr 16, 2025
b3ff4ce
Fix the test cases
Warashi Apr 16, 2025
2405095
Fix the test cases
Warashi Apr 16, 2025
91a7527
Fix the type of the application config in the livestate plugin.
Warashi Apr 16, 2025
b0f18f8
Fix the type of the input for the wait plugin
Warashi Apr 16, 2025
1a0fdbb
Fix the type parameters of the deployment plugin
Warashi Apr 16, 2025
fd6e873
Fix the test in kubernetes_multicluster plugin
Warashi Apr 16, 2025
797e588
Fix the type of the application config in the rollback stage.
Warashi Apr 16, 2025
dccdae5
Fix the type parameters of the plugin SDK.
Warashi Apr 16, 2025
be77e83
Fix the lint notification
Warashi Apr 16, 2025
0cec7a9
Remove empty lines
Warashi Apr 16, 2025
384a0f9
Fix the test
Warashi Apr 16, 2025
e710fd4
Fix the test for DetermineVersionsRequest
Warashi Apr 16, 2025
d152c23
Fix the test for the kubernetes deployment plugin
Warashi Apr 16, 2025
2f52c38
Fix test cases
Warashi Apr 16, 2025
3ed79a2
Fix the livestate test
Warashi Apr 16, 2025
c92050d
Add documentation for the ApplicationConfig type.
Warashi Apr 16, 2025
5abbdd5
Add LoadApplicationConfig and use it in the tests
Warashi Apr 17, 2025
53ef850
Add AppConfig method to DeploymentSource and use it
Warashi Apr 17, 2025
30bd753
Add a helper function to load application config for tests
Warashi Apr 17, 2025
4885a40
Switch to use sdktest.LoadApplicationConfig for tests
Warashi Apr 17, 2025
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
4 changes: 3 additions & 1 deletion pkg/app/pipedv1/plugin/example/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@

type config struct{}

type applicationConfigSpec struct{}

// BuildPipelineSyncStages implements sdk.StagePlugin.
func (p *plugin) BuildPipelineSyncStages(context.Context, *config, *sdk.BuildPipelineSyncStagesInput) (*sdk.BuildPipelineSyncStagesResponse, error) {
return &sdk.BuildPipelineSyncStagesResponse{}, nil
}

// ExecuteStage implements sdk.StagePlugin.
func (p *plugin) ExecuteStage(context.Context, *config, sdk.DeployTargetsNone, *sdk.ExecuteStageInput) (*sdk.ExecuteStageResponse, error) {
func (p *plugin) ExecuteStage(context.Context, *config, sdk.DeployTargetsNone, *sdk.ExecuteStageInput[applicationConfigSpec]) (*sdk.ExecuteStageResponse, error) {

Check warning on line 35 in pkg/app/pipedv1/plugin/example/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/example/plugin.go#L35 

Added line #L35 was not covered by tests
return &sdk.ExecuteStageResponse{}, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/baseline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
"github.com/pipe-cd/pipecd/pkg/plugin/sdk"
)

func (p *Plugin) executeK8sBaselineRolloutStage(_ context.Context, input *sdk.ExecuteStageInput, _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
func (p *Plugin) executeK8sBaselineRolloutStage(_ context.Context, input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec], _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {

Check warning on line 24 in pkg/app/pipedv1/plugin/kubernetes/deployment/baseline.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/baseline.go#L24 

Added line #L24 was not covered by tests
input.Client.LogPersister().Error("Baseline rollout is not yet implemented")
return sdk.StageStatusFailure
}

func (p *Plugin) executeK8sBaselineCleanStage(_ context.Context, input *sdk.ExecuteStageInput, _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
func (p *Plugin) executeK8sBaselineCleanStage(_ context.Context, input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec], _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {

Check warning on line 29 in pkg/app/pipedv1/plugin/kubernetes/deployment/baseline.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/baseline.go#L29 

Added line #L29 was not covered by tests
input.Client.LogPersister().Error("Baseline clean is not yet implemented")
return sdk.StageStatusFailure
}
4 changes: 2 additions & 2 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/canary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
"github.com/pipe-cd/pipecd/pkg/plugin/sdk"
)

func (p *Plugin) executeK8sCanaryRolloutStage(_ context.Context, input *sdk.ExecuteStageInput, _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
func (p *Plugin) executeK8sCanaryRolloutStage(_ context.Context, input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec], _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {

Check warning on line 24 in pkg/app/pipedv1/plugin/kubernetes/deployment/canary.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/canary.go#L24 

Added line #L24 was not covered by tests
input.Client.LogPersister().Error("Canary rollout is not yet implemented")
return sdk.StageStatusFailure
}

func (p *Plugin) executeK8sCanaryCleanStage(_ context.Context, input *sdk.ExecuteStageInput, _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
func (p *Plugin) executeK8sCanaryCleanStage(_ context.Context, input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec], _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {

Check warning on line 29 in pkg/app/pipedv1/plugin/kubernetes/deployment/canary.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/canary.go#L29 

Added line #L29 was not covered by tests
input.Client.LogPersister().Error("Canary clean is not yet implemented")
return sdk.StageStatusFailure
}
38 changes: 17 additions & 21 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
kubeconfig "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/provider"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/toolregistry"
config "github.com/pipe-cd/pipecd/pkg/configv1"
"github.com/pipe-cd/pipecd/pkg/plugin/sdk"
)

Expand All @@ -42,7 +41,7 @@
Helm(ctx context.Context, version string) (string, error)
}

var _ sdk.DeploymentPlugin[sdk.ConfigNone, kubeconfig.KubernetesDeployTargetConfig] = (*Plugin)(nil)
var _ sdk.DeploymentPlugin[sdk.ConfigNone, kubeconfig.KubernetesDeployTargetConfig, kubeconfig.KubernetesApplicationSpec] = (*Plugin)(nil)

// FetchDefinedStages returns the defined stages for this plugin.
func (p *Plugin) FetchDefinedStages() []string {
Expand All @@ -57,7 +56,7 @@
}

// ExecuteStage executes the stage.
func (p *Plugin) ExecuteStage(ctx context.Context, _ *sdk.ConfigNone, dts []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig], input *sdk.ExecuteStageInput) (*sdk.ExecuteStageResponse, error) {
func (p *Plugin) ExecuteStage(ctx context.Context, _ *sdk.ConfigNone, dts []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig], input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec]) (*sdk.ExecuteStageResponse, error) {

Check warning on line 59 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L59 

Added line #L59 was not covered by tests
switch input.Request.StageName {
case StageK8sSync:
return &sdk.ExecuteStageResponse{
Expand Down Expand Up @@ -96,7 +95,7 @@
}
}

func (p *Plugin) loadManifests(ctx context.Context, deploy *sdk.Deployment, spec *kubeconfig.KubernetesApplicationSpec, deploymentSource *sdk.DeploymentSource, loader loader) ([]provider.Manifest, error) {
func (p *Plugin) loadManifests(ctx context.Context, deploy *sdk.Deployment, spec *kubeconfig.KubernetesApplicationSpec, deploymentSource *sdk.DeploymentSource[kubeconfig.KubernetesApplicationSpec], loader loader) ([]provider.Manifest, error) {
manifests, err := loader.LoadManifests(ctx, provider.LoaderInput{
PipedID: deploy.PipedID,
AppID: deploy.ApplicationID,
Expand All @@ -119,17 +118,16 @@
}

// DetermineVersions determines the versions of the application.
func (p *Plugin) DetermineVersions(ctx context.Context, _ *sdk.ConfigNone, input *sdk.DetermineVersionsInput) (*sdk.DetermineVersionsResponse, error) {
func (p *Plugin) DetermineVersions(ctx context.Context, _ *sdk.ConfigNone, input *sdk.DetermineVersionsInput[kubeconfig.KubernetesApplicationSpec]) (*sdk.DetermineVersionsResponse, error) {

Check warning on line 121 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L121 

Added line #L121 was not covered by tests
logger := input.Logger

cfg, err := config.DecodeYAML[*kubeconfig.KubernetesApplicationSpec](input.Request.DeploymentSource.ApplicationConfig)
if err != nil {
logger.Error("Failed while decoding application config", zap.Error(err))
return nil, err
cfg := input.Request.DeploymentSource.ApplicationConfig.Spec
if cfg == nil {
logger.Error("Application config spec is nil")
return nil, errors.New("application config spec is nil")

Check warning on line 127 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L124-L127 

Added lines #L124 - L127 were not covered by tests
}

manifests, err := p.loadManifests(ctx, &input.Request.Deployment, cfg.Spec, &input.Request.DeploymentSource, provider.NewLoader(toolregistry.NewRegistry(input.Client.ToolRegistry())))

manifests, err := p.loadManifests(ctx, &input.Request.Deployment, cfg, &input.Request.DeploymentSource, provider.NewLoader(toolregistry.NewRegistry(input.Client.ToolRegistry())))

Check warning on line 130 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L130 

Added line #L130 was not covered by tests
if err != nil {
logger.Error("Failed while loading manifests", zap.Error(err))
return nil, err
Expand All @@ -141,31 +139,29 @@
}

// DetermineStrategy determines the strategy for the deployment.
func (p *Plugin) DetermineStrategy(ctx context.Context, _ *sdk.ConfigNone, input *sdk.DetermineStrategyInput) (*sdk.DetermineStrategyResponse, error) {
func (p *Plugin) DetermineStrategy(ctx context.Context, _ *sdk.ConfigNone, input *sdk.DetermineStrategyInput[kubeconfig.KubernetesApplicationSpec]) (*sdk.DetermineStrategyResponse, error) {

Check warning on line 142 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L142 

Added line #L142 was not covered by tests
logger := input.Logger
loader := provider.NewLoader(toolregistry.NewRegistry(input.Client.ToolRegistry()))

cfg, err := config.DecodeYAML[*kubeconfig.KubernetesApplicationSpec](input.Request.TargetDeploymentSource.ApplicationConfig)
if err != nil {
logger.Error("Failed while decoding application config", zap.Error(err))
return nil, err
cfg := input.Request.TargetDeploymentSource.ApplicationConfig.Spec
Copy link
Member

Choose a reason for hiding this comment

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

[imo] It would be nice to add a method into DeploySource to get ApplicationConfig from DeploySource, including a nil check because ApplicationConfig is a pointer.
WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with you and I added AppConfig method
53ef850

if cfg == nil {
logger.Error("Application config spec is nil")
return nil, errors.New("application config spec is nil")

Check warning on line 149 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L146-L149 

Added lines #L146 - L149 were not covered by tests
}

runnings, err := p.loadManifests(ctx, &input.Request.Deployment, cfg.Spec, &input.Request.RunningDeploymentSource, loader)

runnings, err := p.loadManifests(ctx, &input.Request.Deployment, cfg, &input.Request.RunningDeploymentSource, loader)

Check warning on line 152 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L152 

Added line #L152 was not covered by tests
if err != nil {
logger.Error("Failed while loading running manifests", zap.Error(err))
return nil, err
}

targets, err := p.loadManifests(ctx, &input.Request.Deployment, cfg.Spec, &input.Request.TargetDeploymentSource, loader)

targets, err := p.loadManifests(ctx, &input.Request.Deployment, cfg, &input.Request.TargetDeploymentSource, loader)

Check warning on line 158 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L158 

Added line #L158 was not covered by tests
if err != nil {
logger.Error("Failed while loading target manifests", zap.Error(err))
return nil, err
}

strategy, summary := determineStrategy(runnings, targets, cfg.Spec.Workloads, logger)
strategy, summary := determineStrategy(runnings, targets, cfg.Workloads, logger)

Check warning on line 164 in pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/plugin.go#L164 

Added line #L164 was not covered by tests

return &sdk.DetermineStrategyResponse{
Strategy: strategy,
Expand Down
2 changes: 1 addition & 1 deletion pkg/app/pipedv1/plugin/kubernetes/deployment/primary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"github.com/pipe-cd/pipecd/pkg/plugin/sdk"
)

func (p *Plugin) executeK8sPrimaryRolloutStage(_ context.Context, input *sdk.ExecuteStageInput, _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
func (p *Plugin) executeK8sPrimaryRolloutStage(_ context.Context, input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec], _ []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {

Check warning on line 24 in pkg/app/pipedv1/plugin/kubernetes/deployment/primary.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/primary.go#L24 

Added line #L24 was not covered by tests
input.Client.LogPersister().Error("Primary rollout is not yet implemented")
return sdk.StageStatusFailure
}
25 changes: 10 additions & 15 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/rollback.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@
kubeconfig "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/provider"
"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/toolregistry"
config "github.com/pipe-cd/pipecd/pkg/configv1"
"github.com/pipe-cd/pipecd/pkg/plugin/sdk"
)

func (p *Plugin) executeK8sRollbackStage(ctx context.Context, input *sdk.ExecuteStageInput, dts []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
func (p *Plugin) executeK8sRollbackStage(ctx context.Context, input *sdk.ExecuteStageInput[kubeconfig.KubernetesApplicationSpec], dts []*sdk.DeployTarget[kubeconfig.KubernetesDeployTargetConfig]) sdk.StageStatus {
lp := input.Client.LogPersister()

if input.Request.RunningDeploymentSource.CommitHash == "" {
Expand All @@ -35,15 +34,11 @@

lp.Info("Start rolling back the deployment")

cfg, err := config.DecodeYAML[*kubeconfig.KubernetesApplicationSpec](input.Request.RunningDeploymentSource.ApplicationConfig)
if err != nil {
lp.Errorf("Failed while decoding application config (%v)", err)
return sdk.StageStatusFailure
}
cfg := input.Request.RunningDeploymentSource.ApplicationConfig.Spec

lp.Infof("Loading manifests at commit %s for handling", input.Request.RunningDeploymentSource.CommitHash)
toolRegistry := toolregistry.NewRegistry(input.Client.ToolRegistry())
manifests, err := p.loadManifests(ctx, &input.Request.Deployment, cfg.Spec, &input.Request.RunningDeploymentSource, provider.NewLoader(toolRegistry))
manifests, err := p.loadManifests(ctx, &input.Request.Deployment, cfg, &input.Request.RunningDeploymentSource, provider.NewLoader(toolRegistry))
if err != nil {
lp.Errorf("Failed while loading manifests (%v)", err)
return sdk.StageStatusFailure
Expand All @@ -57,13 +52,13 @@
// When addVariantLabelToSelector is true, ensure that all workloads
// have the variant label in their selector.
var (
variantLabel = cfg.Spec.VariantLabel.Key
primaryVariant = cfg.Spec.VariantLabel.PrimaryValue
variantLabel = cfg.VariantLabel.Key
primaryVariant = cfg.VariantLabel.PrimaryValue
)
// TODO: Consider other fields to configure whether to add a variant label to the selector
// because the rollback stage is executed in both quick sync and pipeline sync strategies.
if cfg.Spec.QuickSync.AddVariantLabelToSelector {
workloads := findWorkloadManifests(manifests, cfg.Spec.Workloads)
if cfg.QuickSync.AddVariantLabelToSelector {
workloads := findWorkloadManifests(manifests, cfg.Workloads)

Check warning on line 61 in pkg/app/pipedv1/plugin/kubernetes/deployment/rollback.go

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/rollback.go#L61 

Added line #L61 was not covered by tests
for _, m := range workloads {
if err := ensureVariantSelectorInWorkload(m, variantLabel, primaryVariant); err != nil {
lp.Errorf("Unable to check/set %q in selector of workload %s (%v)", variantLabel+": "+primaryVariant, m.Key().ReadableString(), err)
Expand Down Expand Up @@ -95,17 +90,17 @@
deployTargetConfig := dts[0].Config

// Get the kubectl tool path.
kubectlPath, err := toolRegistry.Kubectl(ctx, cmp.Or(cfg.Spec.Input.KubectlVersion, deployTargetConfig.KubectlVersion))
kubectlPath, err := toolRegistry.Kubectl(ctx, cmp.Or(cfg.Input.KubectlVersion, deployTargetConfig.KubectlVersion))
if err != nil {
lp.Errorf("Failed while getting kubectl tool (%v)", err)
return sdk.StageStatusFailure
}

// Create the applier for the target cluster.
applier := provider.NewApplier(provider.NewKubectl(kubectlPath), cfg.Spec.Input, deployTargetConfig, input.Logger)
applier := provider.NewApplier(provider.NewKubectl(kubectlPath), cfg.Input, deployTargetConfig, input.Logger)

// Start applying all manifests to add or update running resources.
if err := applyManifests(ctx, applier, manifests, cfg.Spec.Input.Namespace, lp); err != nil {
if err := applyManifests(ctx, applier, manifests, cfg.Input.Namespace, lp); err != nil {
lp.Errorf("Failed while applying manifests (%v)", err)
return sdk.StageStatusFailure
}
Expand Down
36 changes: 21 additions & 15 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/rollback_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,20 @@ func TestPlugin_executeK8sRollbackStage_NoPreviousDeployment(t *testing.T) {
dtConfig, _ := setupTestDeployTargetConfigAndDynamicClient(t)

// Read the application config from the example file
cfg, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
cfgBytes, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
require.NoError(t, err)
cfg, err := sdk.DecodeYAML[kubeConfigPkg.KubernetesApplicationSpec](cfgBytes)
require.NoError(t, err)

// Prepare the input
input := &sdk.ExecuteStageInput{
Request: sdk.ExecuteStageRequest{
input := &sdk.ExecuteStageInput[kubeConfigPkg.KubernetesApplicationSpec]{
Request: sdk.ExecuteStageRequest[kubeConfigPkg.KubernetesApplicationSpec]{
StageName: "K8S_ROLLBACK",
StageConfig: []byte(``),
RunningDeploymentSource: sdk.DeploymentSource{
RunningDeploymentSource: sdk.DeploymentSource[kubeConfigPkg.KubernetesApplicationSpec]{
CommitHash: "", // Empty commit hash indicates no previous deployment
},
TargetDeploymentSource: sdk.DeploymentSource{
TargetDeploymentSource: sdk.DeploymentSource[kubeConfigPkg.KubernetesApplicationSpec]{
ApplicationDirectory: filepath.Join(examplesDir(), "kubernetes", "simple"),
CommitHash: "0123456789",
ApplicationConfig: cfg,
Expand Down Expand Up @@ -88,21 +90,23 @@ func TestPlugin_executeK8sRollbackStage_SuccessfulRollback(t *testing.T) {
dtConfig, dynamicClient := setupTestDeployTargetConfigAndDynamicClient(t)

// Read the application config from the example file
cfg, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
cfgBytes, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
require.NoError(t, err)
cfg, err := sdk.DecodeYAML[kubeConfigPkg.KubernetesApplicationSpec](cfgBytes)
Copy link
Member

Choose a reason for hiding this comment

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

[imo]
It would be nice to define a helper function to create ApplicationConfigSpec from a file or string.
I feel users don't need to be aware of sdk.DecodeYAML in the actual logic.
WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with you, so I added sdk.LoadApplicationConfig and removed sdk.DecodeYAML
5abbdd5

Copy link
Member

Choose a reason for hiding this comment

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

@Warashi
Thank you for adding the helper method!
I read the comment and think that we can make a test package sdk/sdktest . How about you?

/ LoadApplicationConfig loads the application config from the given filename.
// This is intended to use in the tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ffjlabo

It sounds good.
How about making the sdktest.LoadApplicationConfig 's first argument t *testing.T and automatically calling t.Fatal when it meets an unmarshalling error and making its return value only *ApplicationConfig[Spec]?

Copy link
Member

Choose a reason for hiding this comment

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

@Warashi Sounds great!!! Let's go with that!

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented and switched the usage.
30bd753
4885a40

Copy link
Member

Choose a reason for hiding this comment

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

Sounds nice! That's a nice improvement!

require.NoError(t, err)

// Prepare the input
input := &sdk.ExecuteStageInput{
Request: sdk.ExecuteStageRequest{
input := &sdk.ExecuteStageInput[kubeConfigPkg.KubernetesApplicationSpec]{
Request: sdk.ExecuteStageRequest[kubeConfigPkg.KubernetesApplicationSpec]{
StageName: "K8S_ROLLBACK",
StageConfig: []byte(``),
RunningDeploymentSource: sdk.DeploymentSource{
RunningDeploymentSource: sdk.DeploymentSource[kubeConfigPkg.KubernetesApplicationSpec]{
ApplicationDirectory: filepath.Join(examplesDir(), "kubernetes", "simple"),
CommitHash: "previous-hash",
ApplicationConfig: cfg,
ApplicationConfigFilename: "app.pipecd.yaml",
},
TargetDeploymentSource: sdk.DeploymentSource{
TargetDeploymentSource: sdk.DeploymentSource[kubeConfigPkg.KubernetesApplicationSpec]{
ApplicationDirectory: filepath.Join(examplesDir(), "kubernetes", "simple"),
CommitHash: "0123456789",
ApplicationConfig: cfg,
Expand Down Expand Up @@ -155,21 +159,23 @@ func TestPlugin_executeK8sRollbackStage_WithVariantLabels(t *testing.T) {
dtConfig, dynamicClient := setupTestDeployTargetConfigAndDynamicClient(t)

// Read the application config and modify it to include variant labels
cfg, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
cfgBytes, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
require.NoError(t, err)
cfg, err := sdk.DecodeYAML[kubeConfigPkg.KubernetesApplicationSpec](cfgBytes)
require.NoError(t, err)

// Prepare the input
input := &sdk.ExecuteStageInput{
Request: sdk.ExecuteStageRequest{
input := &sdk.ExecuteStageInput[kubeConfigPkg.KubernetesApplicationSpec]{
Request: sdk.ExecuteStageRequest[kubeConfigPkg.KubernetesApplicationSpec]{
StageName: "K8S_ROLLBACK",
StageConfig: []byte(``),
RunningDeploymentSource: sdk.DeploymentSource{
RunningDeploymentSource: sdk.DeploymentSource[kubeConfigPkg.KubernetesApplicationSpec]{
ApplicationDirectory: filepath.Join(examplesDir(), "kubernetes", "simple"),
CommitHash: "previous-hash",
ApplicationConfig: cfg,
ApplicationConfigFilename: "app.pipecd.yaml",
},
TargetDeploymentSource: sdk.DeploymentSource{
TargetDeploymentSource: sdk.DeploymentSource[kubeConfigPkg.KubernetesApplicationSpec]{
ApplicationDirectory: filepath.Join(examplesDir(), "kubernetes", "simple"),
CommitHash: "0123456789",
ApplicationConfig: cfg,
Expand Down
Loading
Loading