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

add param timeout for helm target provider #435

Merged
merged 10 commits into from
Sep 2, 2024
Merged

add param timeout for helm target provider #435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2abf693
add param timeout for helm target provider
yanjiaxin534 Aug 28, 2024
f5a1e8c
add ctx to log
yanjiaxin534 Aug 28, 2024
014c18d
fix build issue
yanjiaxin534 Aug 29, 2024
0855a25
Merge branch 'main' into add-timeout-param-for-helm
msftcoderdjw Aug 29, 2024
172a450
Merge branch 'main' into add-timeout-param-for-helm
msftcoderdjw Aug 29, 2024
d11914a
add ut for timeout param
yanjiaxin534 Aug 29, 2024
7d03084
add ut for timeout param
yanjiaxin534 Aug 29, 2024
7dc3076
add ut for timeout param
yanjiaxin534 Aug 29, 2024
36f090c
add ut for timeout param
yanjiaxin534 Aug 29, 2024
4970549
add ut for timeout param
yanjiaxin534 Aug 29, 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
70 changes: 58 additions & 12 deletions api/pkg/apis/v1alpha1/providers/target/helm/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ type (
Name string `json:"name,omitempty"`
Version string `json:"version"`
Wait bool `json:"wait"`
Timeout string `json:"timeout,omitempty"`
}
)

Expand Down Expand Up @@ -442,9 +443,9 @@ func (i *HelmTargetProvider) Apply(ctx context.Context, deployment model.Deploym

for _, component := range step.Components {
applyComponentTime := time.Now().UTC()
var helmProp *HelmProperty
helmProp, err = getHelmPropertyFromComponent(component.Component)
if component.Action == model.ComponentUpdate {
var helmProp *HelmProperty
helmProp, err = getHelmPropertyFromComponent(component.Component)
if err != nil {
sLog.ErrorfCtx(ctx, " P (Helm Target): failed to get Helm properties: %+v", err)
err = v1alpha2.NewCOAError(err, fmt.Sprintf("%s: failed to get helm properties", providerName), v1alpha2.GetHelmPropertyFailed)
Expand Down Expand Up @@ -510,9 +511,14 @@ func (i *HelmTargetProvider) Apply(ctx context.Context, deployment model.Deploym
instance: deployment.Instance,
populator: i.MetaPopulator,
}
installClient := configureInstallClient(component.Component.Name, &helmProp.Chart, &deployment, actionConfig, postRender)
upgradeClient := configureUpgradeClient(&helmProp.Chart, &deployment, actionConfig, postRender)

installClient, err := configureInstallClient(ctx, component.Component.Name, &helmProp.Chart, &deployment, actionConfig, postRender)
if err != nil {
return nil, err
}
upgradeClient, err := configureUpgradeClient(ctx, &helmProp.Chart, &deployment, actionConfig, postRender)
if err != nil {
return nil, err
}
utils.EmitUserAuditsLogs(ctx, " P (Helm Target): Applying chart name: %s, chart: {repo: %s, name: %s, version: %s}, namespace: %s", component.Component.Name, helmProp.Chart.Repo, helmProp.Chart.Name, helmProp.Chart.Version, deployment.Instance.Spec.Scope)
if _, err = upgradeClient.Run(component.Component.Name, chart, helmProp.Values); err != nil {
if _, err = installClient.Run(chart, helmProp.Values); err != nil {
Expand Down Expand Up @@ -549,7 +555,10 @@ func (i *HelmTargetProvider) Apply(ctx context.Context, deployment model.Deploym
} else {
switch component.Component.Type {
case "helm.v3":
uninstallClient := configureUninstallClient(&deployment, actionConfig)
uninstallClient, err := configureUninstallClient(ctx, &helmProp.Chart, &deployment, actionConfig)
if err != nil {
return nil, err
}
utils.EmitUserAuditsLogs(ctx, " P (Helm Target): Uninstalling chart name: %s, namespace: %s", component.Component.Name, deployment.Instance.Spec.Scope)
_, err = uninstallClient.Run(component.Component.Name)
if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
Expand Down Expand Up @@ -680,26 +689,42 @@ func pullOCIChart(ctx context.Context, repo, version string) (*registry.PullResu
return pullRes, nil
}

func configureInstallClient(name string, componentProps *HelmChartProperty, deployment *model.DeploymentSpec, config *action.Configuration, postRenderer postrender.PostRenderer) *action.Install {
func configureInstallClient(ctx context.Context, name string, componentProps *HelmChartProperty, deployment *model.DeploymentSpec, config *action.Configuration, postRenderer postrender.PostRenderer) (*action.Install, error) {
installClient := action.NewInstall(config)
installClient.ReleaseName = name
if deployment.Instance.Spec.Scope == "" {
installClient.Namespace = constants.DefaultScope
} else {
installClient.Namespace = deployment.Instance.Spec.Scope
}

installClient.Wait = componentProps.Wait
if componentProps.Timeout != "" {
duration, err := convertTimeout(ctx, componentProps.Timeout)
if err != nil {
return nil, err
}
installClient.Timeout = duration
}

installClient.IsUpgrade = true
installClient.CreateNamespace = true
installClient.PostRenderer = postRenderer
// We can't add labels to the release in the current version of the helm client.
// This should added when we upgrade to helm ^3.13.1
return installClient
return installClient, nil
}

func configureUpgradeClient(componentProps *HelmChartProperty, deployment *model.DeploymentSpec, config *action.Configuration, postRenderer postrender.PostRenderer) *action.Upgrade {
func configureUpgradeClient(ctx context.Context, componentProps *HelmChartProperty, deployment *model.DeploymentSpec, config *action.Configuration, postRenderer postrender.PostRenderer) (*action.Upgrade, error) {
upgradeClient := action.NewUpgrade(config)
upgradeClient.Wait = componentProps.Wait
if componentProps.Timeout != "" {
duration, err := convertTimeout(ctx, componentProps.Timeout)
if err != nil {
return nil, err
}
upgradeClient.Timeout = duration
}
if deployment.Instance.Spec.Scope == "" {
upgradeClient.Namespace = constants.DefaultScope
} else {
Expand All @@ -710,12 +735,33 @@ func configureUpgradeClient(componentProps *HelmChartProperty, deployment *model
upgradeClient.PostRenderer = postRenderer
// We can't add labels to the release in the current version of the helm client.
// This should added when we upgrade to helm ^3.13.1
return upgradeClient
return upgradeClient, nil
}

func configureUninstallClient(deployment *model.DeploymentSpec, config *action.Configuration) *action.Uninstall {
func configureUninstallClient(ctx context.Context, componentProps *HelmChartProperty, deployment *model.DeploymentSpec, config *action.Configuration) (*action.Uninstall, error) {
uninstallClient := action.NewUninstall(config)
return uninstallClient
uninstallClient.Wait = componentProps.Wait
yanjiaxin534 marked this conversation as resolved.
Show resolved Hide resolved
if componentProps.Timeout != "" {
duration, err := convertTimeout(ctx, componentProps.Timeout)
if err != nil {
return nil, err
}
uninstallClient.Timeout = duration
}
return uninstallClient, nil
}

func convertTimeout(ctx context.Context, timeout string) (time.Duration, error) {
duration, err := time.ParseDuration(timeout)
if err != nil {
sLog.ErrorfCtx(ctx, " P (Helm Target): failed to parse timeout duration: %v", err)
return 0, err
}
if duration < 0 {
sLog.ErrorfCtx(ctx, " P (Helm Target): Timeout is negative: %s", timeout)
return 0, errors.New("Timeout can not be negative.")
}
return duration, nil
}

func getHelmPropertyFromComponent(component model.ComponentSpec) (*HelmProperty, error) {
Expand Down
141 changes: 141 additions & 0 deletions api/pkg/apis/v1alpha1/providers/target/helm/helm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -621,6 +622,146 @@ func TestHelmTargetProviderUpdateDelete(t *testing.T) {
assert.Nil(t, err)
}

func TestHelmTargetProviderWithNegativeTimeout(t *testing.T) {
testEnabled := os.Getenv("TEST_MINIKUBE_ENABLED")
if testEnabled == "" {
t.Skip("Skipping because TEST_MINIKUBE_ENABLED enviornment variable is not set")
}
config := HelmTargetProviderConfig{InCluster: true}
provider := HelmTargetProvider{}
err := provider.Init(config)
assert.Nil(t, err)
component := model.ComponentSpec{
Name: "nginx",
Type: "helm.v3",
Properties: map[string]interface{}{
"chart": map[string]any{
"repo": "https://github.com/kubernetes/ingress-nginx/releases/download/helm-chart-4.7.1/ingress-nginx-4.7.1.tgz",
"name": "nginx",
"wait": true,
"timeout": "-10s",
},
},
}
deployment := model.DeploymentSpec{
Instance: model.InstanceState{
Spec: &model.InstanceSpec{},
},
Solution: model.SolutionState{
Spec: &model.SolutionSpec{
Components: []model.ComponentSpec{component},
},
},
}
step := model.DeploymentStep{
Components: []model.ComponentStep{
{
Action: model.ComponentUpdate,
Component: component,
},
},
}
_, err = provider.Apply(context.Background(), deployment, step, false)
fmt.Printf("error timeout %v", err.Error())
if !strings.Contains(err.Error(), "Timeout can not be negative") {
t.Errorf("expected error to contain 'Timeout can not be negative', but got %s", err.Error())
}
assert.NotNil(t, err)
}

func TestHelmTargetProviderWithPositiveTimeout(t *testing.T) {
yanjiaxin534 marked this conversation as resolved.
Show resolved Hide resolved
testEnabled := os.Getenv("TEST_MINIKUBE_ENABLED")
if testEnabled == "" {
t.Skip("Skipping because TEST_MINIKUBE_ENABLED enviornment variable is not set")
}
config := HelmTargetProviderConfig{InCluster: true}
provider := HelmTargetProvider{}
err := provider.Init(config)
assert.Nil(t, err)
component := model.ComponentSpec{
Name: "brigade",
Type: "helm.v3",
Properties: map[string]interface{}{
"chart": map[string]any{
"repo": "https://brigadecore.github.io/charts",
"name": "brigade",
"wait": true,
"timeout": "0.01s",
},
},
}
deployment := model.DeploymentSpec{
Instance: model.InstanceState{
Spec: &model.InstanceSpec{},
},
Solution: model.SolutionState{
Spec: &model.SolutionSpec{
Components: []model.ComponentSpec{component},
},
},
}
step := model.DeploymentStep{
Components: []model.ComponentStep{
{
Action: model.ComponentUpdate,
Component: component,
},
},
}
_, err = provider.Apply(context.Background(), deployment, step, false)
if !strings.Contains(err.Error(), "context deadline exceeded") {
t.Errorf("expected error to contain 'context deadline exceeded', but got %s", err.Error())
}
assert.NotNil(t, err)
}

func TestHelmTargetProviderWithInvalidTimeout(t *testing.T) {
os.Setenv("TEST_MINIKUBE_ENABLED", "yes")
testEnabled := os.Getenv("TEST_MINIKUBE_ENABLED")
if testEnabled == "" {
t.Skip("Skipping because TEST_MINIKUBE_ENABLED enviornment variable is not set")
}
config := HelmTargetProviderConfig{InCluster: true}
provider := HelmTargetProvider{}
err := provider.Init(config)
assert.Nil(t, err)
component := model.ComponentSpec{
Name: "brigade",
Type: "helm.v3",
Properties: map[string]interface{}{
"chart": map[string]any{
"repo": "https://brigadecore.github.io/charts",
"name": "brigade",
"wait": true,
"timeout": "20ssss",
},
},
}
deployment := model.DeploymentSpec{
Instance: model.InstanceState{
Spec: &model.InstanceSpec{},
},
Solution: model.SolutionState{
Spec: &model.SolutionSpec{
Components: []model.ComponentSpec{component},
},
},
}
step := model.DeploymentStep{
Components: []model.ComponentStep{
{
Action: model.ComponentUpdate,
Component: component,
},
},
}
_, err = provider.Apply(context.Background(), deployment, step, false)
if !strings.Contains(err.Error(), "time: unknown unit ") {
t.Errorf("expected error to contain 'time: unknown unit', but got %s", err.Error())
}
assert.NotNil(t, err)
}

func TestHelmTargetProviderUpdateFailed(t *testing.T) {
testEnabled := os.Getenv("TEST_MINIKUBE_ENABLED")
if testEnabled == "" {
Expand Down
Loading