Skip to content

Commit

Permalink
fix: handle for wrong format of k8s version in semvercompare func in …
Browse files Browse the repository at this point in the history
…cronjob template charts (#5016)

* handle for wrong format of k8s version in semvercompare func in cronjob template charts

* TestStripPrereleaseFromK8sVersion UT's added

* constants added

* incorporated code review changes

* merge main
  • Loading branch information
prakash100198 authored Apr 30, 2024
1 parent b311592 commit fe2ffb2
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 12 deletions.
28 changes: 24 additions & 4 deletions api/helm-app/gRPC/applist.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/helm-app/gRPC/applist.proto
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ message UpgradeReleaseRequest {
int32 historyMax = 3;
ChartContent chartContent = 4;
bool RunInCtx = 5;
string K8sVersion = 6;
}

message UpgradeReleaseResponse {
Expand Down Expand Up @@ -317,6 +318,7 @@ message HelmInstallCustomRequest {
ChartContent chartContent = 2;
ReleaseIdentifier releaseIdentifier = 3;
bool RunInCtx = 4;
string K8sVersion = 5;
}

message HelmInstallCustomResponse {
Expand Down
2 changes: 2 additions & 0 deletions api/helm-app/service/HelmAppService.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ type HelmAppService interface {
GetDesiredManifest(ctx context.Context, app *AppIdentifier, resource *openapi.ResourceIdentifier) (*openapi.DesiredManifestResponse, error)
DeleteApplication(ctx context.Context, app *AppIdentifier) (*openapi.UninstallReleaseResponse, error)
DeleteDBLinkedHelmApplication(ctx context.Context, app *AppIdentifier, useId int32) (*openapi.UninstallReleaseResponse, error)
// UpdateApplication is a wrapper over helmAppClient.UpdateApplication, sends update request to kubelink for external chart store apps
UpdateApplication(ctx context.Context, app *AppIdentifier, request *bean.UpdateApplicationRequestDto) (*openapi.UpdateReleaseResponse, error)
GetDeploymentDetail(ctx context.Context, app *AppIdentifier, version int32) (*openapi.HelmAppDeploymentManifestDetail, error)
InstallRelease(ctx context.Context, clusterId int, installReleaseRequest *gRPC.InstallReleaseRequest) (*gRPC.InstallReleaseResponse, error)
// UpdateApplicationWithChartInfo is a wrapper over helmAppClient.UpdateApplicationWithChartInfo sends update request to kubelink for helm chart store apps
UpdateApplicationWithChartInfo(ctx context.Context, clusterId int, request *bean.UpdateApplicationWithChartInfoRequestDto) (*openapi.UpdateReleaseResponse, error)
IsReleaseInstalled(ctx context.Context, app *AppIdentifier) (bool, error)
RollbackRelease(ctx context.Context, app *AppIdentifier, version int32) (bool, error)
Expand Down
4 changes: 2 additions & 2 deletions env_gen.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@
| REQ_CI_MEM | 3G | |
| RESOURCE_LIST_FOR_REPLICAS | Deployment,Rollout,StatefulSet,ReplicaSet | |
| RESOURCE_LIST_FOR_REPLICAS_BATCH_SIZE | 5 | |
| REVISION_HISTORY_LIMIT_DEVTRON_APP | 0 | |
| REVISION_HISTORY_LIMIT_DEVTRON_APP | 1 | |
| REVISION_HISTORY_LIMIT_EXTERNAL_HELM_APP | 0 | |
| REVISION_HISTORY_LIMIT_HELM_APP | 0 | |
| REVISION_HISTORY_LIMIT_HELM_APP | 1 | |
| RUNTIME_CONFIG_LOCAL_DEV | false | |
| RUN_HELM_INSTALL_IN_ASYNC_MODE_HELM_APPS | false | |
| SCOPED_VARIABLE_ENABLED | false | |
Expand Down
33 changes: 29 additions & 4 deletions pkg/deployment/trigger/devtronApps/TriggerService.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
"github.com/devtron-labs/devtron/pkg/eventProcessor/out"
"github.com/devtron-labs/devtron/pkg/imageDigestPolicy"
k8s2 "github.com/devtron-labs/devtron/pkg/k8s"
"github.com/devtron-labs/devtron/pkg/pipeline"
bean8 "github.com/devtron-labs/devtron/pkg/pipeline/bean"
"github.com/devtron-labs/devtron/pkg/pipeline/history"
Expand All @@ -64,6 +65,7 @@ import (
"k8s.io/helm/pkg/proto/hapi/chart"
"net/http"
"path"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -137,6 +139,7 @@ type TriggerServiceImpl struct {
ciPipelineRepository pipelineConfig.CiPipelineRepository
appWorkflowRepository appWorkflow.AppWorkflowRepository
dockerArtifactStoreRepository repository4.DockerArtifactStoreRepository
K8sUtil *util5.K8sServiceImpl
}

func NewTriggerServiceImpl(logger *zap.SugaredLogger, cdWorkflowCommonService cd.CdWorkflowCommonService,
Expand Down Expand Up @@ -186,7 +189,8 @@ func NewTriggerServiceImpl(logger *zap.SugaredLogger, cdWorkflowCommonService cd
ciPipelineRepository pipelineConfig.CiPipelineRepository,
appWorkflowRepository appWorkflow.AppWorkflowRepository,
dockerArtifactStoreRepository repository4.DockerArtifactStoreRepository,
imageScanService security2.ImageScanService) (*TriggerServiceImpl, error) {
imageScanService security2.ImageScanService,
K8sUtil *util5.K8sServiceImpl) (*TriggerServiceImpl, error) {
impl := &TriggerServiceImpl{
logger: logger,
cdWorkflowCommonService: cdWorkflowCommonService,
Expand Down Expand Up @@ -237,6 +241,7 @@ func NewTriggerServiceImpl(logger *zap.SugaredLogger, cdWorkflowCommonService cd
appWorkflowRepository: appWorkflowRepository,
dockerArtifactStoreRepository: dockerArtifactStoreRepository,
imageScanService: imageScanService,
K8sUtil: K8sUtil,
}
config, err := types.GetCdConfig()
if err != nil {
Expand Down Expand Up @@ -877,9 +882,23 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean
Name: pipeline.App.AppName,
Version: envOverride.Chart.ChartVersion,
}
referenceTemplatePath := path.Join(bean5.RefChartDirPath, envOverride.Chart.ReferenceTemplate)
referenceTemplate := envOverride.Chart.ReferenceTemplate
referenceTemplatePath := path.Join(bean5.RefChartDirPath, referenceTemplate)

if util.IsHelmApp(pipeline.DeploymentAppType) {
var sanitizedK8sVersion string
//handle specific case for all cronjob charts from cronjob-chart_1-2-0 to cronjob-chart_1-5-0 where semverCompare
//comparison func has wrong api version mentioned, so for already installed charts via these charts that comparison
//is always false, handles the gh issue:- https://github.com/devtron-labs/devtron/issues/4860
cronJobChartRegex := regexp.MustCompile(bean.CronJobChartRegexExpression)
if cronJobChartRegex.MatchString(referenceTemplate) {
k8sServerVersion, err := impl.K8sUtil.GetKubeVersion()
if err != nil {
impl.logger.Errorw("exception caught in getting k8sServerVersion", "err", err)
return false, err
}
sanitizedK8sVersion = k8s2.StripPrereleaseFromK8sVersion(k8sServerVersion.String())
}
referenceChartByte := envOverride.Chart.ReferenceChart
// here updating reference chart into database.
if len(envOverride.Chart.ReferenceChart) == 0 {
Expand Down Expand Up @@ -927,6 +946,9 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean
HistoryMax: impl.helmAppService.GetRevisionHistoryMaxValue(bean6.SOURCE_DEVTRON_APP),
ChartContent: &gRPC.ChartContent{Content: referenceChartByte},
}
if len(sanitizedK8sVersion) > 0 {
req.K8SVersion = sanitizedK8sVersion
}
if impl.isDevtronAsyncInstallModeEnabled(bean.Helm) {
req.RunInCtx = true
}
Expand All @@ -948,7 +970,7 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean

} else {

helmResponse, err := impl.helmInstallReleaseWithCustomChart(ctx, releaseIdentifier, referenceChartByte, mergeAndSave)
helmResponse, err := impl.helmInstallReleaseWithCustomChart(ctx, releaseIdentifier, referenceChartByte, mergeAndSave, sanitizedK8sVersion)

// For connection related errors, no need to update the db
if err != nil && strings.Contains(err.Error(), "connection error") {
Expand Down Expand Up @@ -1154,13 +1176,16 @@ func (impl *TriggerServiceImpl) updatePipeline(pipeline *pipelineConfig.Pipeline
}

// helmInstallReleaseWithCustomChart performs helm install with custom chart
func (impl *TriggerServiceImpl) helmInstallReleaseWithCustomChart(ctx context.Context, releaseIdentifier *gRPC.ReleaseIdentifier, referenceChartByte []byte, valuesYaml string) (*gRPC.HelmInstallCustomResponse, error) {
func (impl *TriggerServiceImpl) helmInstallReleaseWithCustomChart(ctx context.Context, releaseIdentifier *gRPC.ReleaseIdentifier, referenceChartByte []byte, valuesYaml string, k8sServerVersion string) (*gRPC.HelmInstallCustomResponse, error) {

helmInstallRequest := gRPC.HelmInstallCustomRequest{
ValuesYaml: valuesYaml,
ChartContent: &gRPC.ChartContent{Content: referenceChartByte},
ReleaseIdentifier: releaseIdentifier,
}
if len(k8sServerVersion) > 0 {
helmInstallRequest.K8SVersion = k8sServerVersion
}
if impl.isDevtronAsyncInstallModeEnabled(bean.Helm) {
helmInstallRequest.RunInCtx = true
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/deployment/trigger/devtronApps/bean/bean.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,7 @@ type VulnerabilityCheckRequest struct {
ImageDigest string
CdPipeline *pipelineConfig.Pipeline
}

const (
CronJobChartRegexExpression = "cronjob-chart_1-(2|3|4|5)-0"
)
15 changes: 14 additions & 1 deletion pkg/generateManifest/DeployementTemplateService.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/devtron-labs/devtron/pkg/chart"
repository3 "github.com/devtron-labs/devtron/pkg/cluster/repository"
"github.com/devtron-labs/devtron/pkg/deployment/manifest/deploymentTemplate/chartRef"
bean2 "github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/bean"
k8s2 "github.com/devtron-labs/devtron/pkg/k8s"
"github.com/devtron-labs/devtron/pkg/pipeline"
"github.com/devtron-labs/devtron/pkg/pipeline/history"
"github.com/devtron-labs/devtron/pkg/resourceQualifiers"
Expand All @@ -25,6 +27,7 @@ import (
"go.uber.org/zap"
"net/http"
"os"
"regexp"
"strconv"
"time"
)
Expand Down Expand Up @@ -336,11 +339,21 @@ func (impl DeploymentTemplateServiceImpl) GenerateManifest(ctx context.Context,
impl.Logger.Errorw("exception caught in getting k8sServerVersion", "err", err)
return nil, err
}

sanitizedK8sVersion := k8sServerVersion.String()
//handle specific case for all cronjob charts from cronjob-chart_1-2-0 to cronjob-chart_1-5-0 where semverCompare
//comparison func has wrong api version mentioned, so for already installed charts via these charts that comparison
//is always false, handles the gh issue:- https://github.com/devtron-labs/devtron/issues/4860
cronJobChartRegex := regexp.MustCompile(bean2.CronJobChartRegexExpression)
if cronJobChartRegex.MatchString(template) {
sanitizedK8sVersion = k8s2.StripPrereleaseFromK8sVersion(sanitizedK8sVersion)
}

installReleaseRequest := &gRPC.InstallReleaseRequest{
ChartName: template,
ChartVersion: version,
ValuesYaml: valuesYaml,
K8SVersion: k8sServerVersion.String(),
K8SVersion: sanitizedK8sVersion,
ChartRepository: ChartRepository,
ReleaseIdentifier: ReleaseIdentifier,
ChartContent: &gRPC.ChartContent{
Expand Down
18 changes: 18 additions & 0 deletions pkg/k8s/helper.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package k8s

import (
"fmt"
"github.com/Masterminds/semver"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"strings"
)

func IsResourceNotFoundErr(err error) bool {
Expand All @@ -11,3 +14,18 @@ func IsResourceNotFoundErr(err error) bool {
}
return false
}

// StripPrereleaseFromK8sVersion takes in k8sVersion and stripe pre-release from semver version and return sanitized k8sVersion
// or error if invalid version provided, e.g. if k8sVersion = "1.25.16-eks-b9c9ed7", then it returns "1.25.16".
func StripPrereleaseFromK8sVersion(k8sVersion string) string {
version, err := semver.NewVersion(k8sVersion)
if err != nil {
fmt.Printf("error in stripping pre-release from k8sServerVersion due to invalid k8sServerVersion:= %s, err:= %v", k8sVersion, err)
return k8sVersion
}
if len(version.Prerelease()) > 0 {
stringToReplace := "-" + version.Prerelease()
k8sVersion = strings.Replace(k8sVersion, stringToReplace, "", 1)
}
return k8sVersion
}
57 changes: 57 additions & 0 deletions pkg/k8s/helper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package k8s

import "testing"

// not removing metadata from k8s version if exists, we only eliminate pre-release from k8s version
const (
K8sVersionWithPreRelease = "v1.25.16-eks-b9c9ed7"
K8sVersionWithPreReleaseAndMetadata = "v1.25.16-eks-b9c9ed7+acj23-as"
K8sVersionWithMetadata = "v1.25.16+acj23-as"
K8sVersionWithoutPreReleaseAndMetadata = "v1.25.16"
InvalidK8sVersion = ""
)

func TestStripPrereleaseFromK8sVersion(t *testing.T) {
type args struct {
k8sVersion string
}
tests := []struct {
name string
args args
want string
}{
{
name: "Test1_K8sVersionWithPreRelease",
args: args{k8sVersion: K8sVersionWithPreRelease},
want: K8sVersionWithoutPreReleaseAndMetadata,
},
{
name: "Test2_K8sVersionWithPreReleaseAndMetadata",
args: args{k8sVersion: K8sVersionWithPreReleaseAndMetadata},
want: K8sVersionWithMetadata,
},
{
name: "Test3_K8sVersionWithMetadata",
args: args{k8sVersion: K8sVersionWithMetadata},
want: K8sVersionWithMetadata,
},
{
name: "Test4_K8sVersionWithoutPrereleaseAndMetadata",
args: args{k8sVersion: K8sVersionWithoutPreReleaseAndMetadata},
want: K8sVersionWithoutPreReleaseAndMetadata,
},
{
name: "Test5_EmptyK8sVersion",
args: args{k8sVersion: InvalidK8sVersion},
want: InvalidK8sVersion,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := StripPrereleaseFromK8sVersion(tt.args.k8sVersion)
if got != tt.want {
t.Errorf("StripPrereleaseFromK8sVersion() got = %v, want %v", got, tt.want)
}
})
}
}
Loading

0 comments on commit fe2ffb2

Please sign in to comment.