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.

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

Pass DeployTarget in SDK #5616

Merged
merged 8 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
6 changes: 3 additions & 3 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@
}

// Get the deploy target config.
targets, err := input.GetDeployment().GetDeployTargets(a.pluginConfig.Name)
if err != nil {
lp.Errorf("Failed while finding deploy target config (%v)", err)
targets := input.GetDeployment().GetDeployTargets(a.pluginConfig.Name)
if len(targets) == 0 {
lp.Errorf("No deploy target was found for the plugin %s", a.pluginConfig.Name)

Check warning on line 88 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#L86-L88

Added lines #L86 - L88 were not covered by tests
return model.StageStatus_STAGE_FAILURE
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@
}

// Get the deploy target config.
targets, err := input.GetDeployment().GetDeployTargets(a.pluginConfig.Name)
if err != nil {
lp.Errorf("Failed while finding deploy target config (%v)", err)
targets := input.GetDeployment().GetDeployTargets(a.pluginConfig.Name)
if len(targets) == 0 {
lp.Errorf("No deploy target was found for the plugin %s", a.pluginConfig.Name)

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

View check run for this annotation

Codecov / codecov/patch

pkg/app/pipedv1/plugin/kubernetes/deployment/sync.go#L87

Added line #L87 was not covered by tests
return model.StageStatus_STAGE_FAILURE
}
deployTargetConfig, err := kubeconfig.FindDeployTarget(a.pluginConfig, targets[0]) // TODO: consider multiple targets
Expand Down
6 changes: 3 additions & 3 deletions pkg/model/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,13 @@ func (d *Deployment) SetUpdatedAt(t int64) {
d.UpdatedAt = t
}

func (d *Deployment) GetDeployTargets(pluginName string) ([]string, error) {
func (d *Deployment) GetDeployTargets(pluginName string) []string {
dps, ok := d.GetDeployTargetsByPlugin()[pluginName]
if !ok || len(dps.GetDeployTargets()) == 0 {
return nil, fmt.Errorf("deploy target not found for plugin %v", pluginName)
return []string{}
}

return dps.GetDeployTargets(), nil
return dps.GetDeployTargets()
}

// Implement sort.Interface for PipelineStages.
Expand Down
65 changes: 65 additions & 0 deletions pkg/model/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,3 +658,68 @@ func TestSortPipelineStagesByIndex(t *testing.T) {
{Index: 4},
}, stages)
}

func TestDeployment_GetDeployTargets(t *testing.T) {
t.Parallel()

tests := []struct {
name string
deployment *Deployment
pluginName string
expected []string
}{
{
name: "plugin with deploy targets",
deployment: &Deployment{
DeployTargetsByPlugin: map[string]*DeployTargets{
"pluginA": {
DeployTargets: []string{"target1", "target2"},
},
},
},
pluginName: "pluginA",
expected: []string{"target1", "target2"},
},
{
name: "plugin without deploy targets",
deployment: &Deployment{
DeployTargetsByPlugin: map[string]*DeployTargets{
"pluginA": {
DeployTargets: []string{},
},
},
},
pluginName: "pluginA",
expected: []string{},
},
{
name: "plugin not found",
deployment: &Deployment{
DeployTargetsByPlugin: map[string]*DeployTargets{
"pluginA": {
DeployTargets: []string{"target1", "target2"},
},
},
},
pluginName: "pluginB",
expected: []string{},
},
{
name: "no deploy targets by plugin",
deployment: &Deployment{
DeployTargetsByPlugin: map[string]*DeployTargets{},
},
pluginName: "pluginA",
expected: []string{},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

got := tt.deployment.GetDeployTargets(tt.pluginName)
assert.Equal(t, tt.expected, got)
})
}
}
29 changes: 28 additions & 1 deletion pkg/plugin/sdk/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,34 @@
stageID: request.GetInput().GetStage().GetId(),
logPersister: lp,
}
return executeStage(ctx, s.base, &s.config, nil, client, request, s.logger) // TODO: pass the deployTargets

// Get the deploy targets set on the deployment from the piped plugin config.
dtNames := request.GetInput().GetDeployment().GetDeployTargets(s.commonFields.config.Name)
deployTargets := make([]*DeployTarget[DeployTargetConfig], 0, len(dtNames))
for _, name := range dtNames {
dt := s.commonFields.config.FindDeployTarget(name)
if dt == nil {
continue

Check warning on line 216 in pkg/plugin/sdk/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/plugin/sdk/deployment.go#L209-L216

Added lines #L209 - L216 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

[IMO]
How about returning an error here? If we return here, we can remove the if len(dtNames) != len(deployTargets) block after the for loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Warashi
Ah, finally I got your point.
Fixed on 1efc476

}

// TODO: cache the unmarshaled config to avoid unmarshaling it multiple times.
var sdkDt DeployTargetConfig
if err := json.Unmarshal(dt.Config, &sdkDt); err != nil {
return nil, status.Errorf(codes.Internal, "failed to unmarshal deploy target config: %v", err)
}

Check warning on line 223 in pkg/plugin/sdk/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/plugin/sdk/deployment.go#L220-L223

Added lines #L220 - L223 were not covered by tests

deployTargets = append(deployTargets, &DeployTarget[DeployTargetConfig]{
Name: name,
Labels: dt.Labels,
Config: sdkDt,
})

Check warning on line 229 in pkg/plugin/sdk/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/plugin/sdk/deployment.go#L225-L229

Added lines #L225 - L229 were not covered by tests
}

if len(dtNames) != len(deployTargets) {
return nil, status.Errorf(codes.Internal, "the number of deploy targets in the piped plugin config should be the same as the ones set on the deployment: in the piped config = %d, in the deployment= %d", len(dtNames), len(deployTargets))
}

Check warning on line 234 in pkg/plugin/sdk/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/plugin/sdk/deployment.go#L232-L234

Added lines #L232 - L234 were not covered by tests

return executeStage(ctx, s.base, &s.config, deployTargets, client, request, s.logger)

Check warning on line 236 in pkg/plugin/sdk/deployment.go

View check run for this annotation

Codecov / codecov/patch

pkg/plugin/sdk/deployment.go#L236

Added line #L236 was not covered by tests
}

// StagePluginServiceServer is the gRPC server that handles requests from the piped.
Expand Down
Loading