Skip to content

Commit fe2ffb2

Browse files
fix: handle for wrong format of k8s version in semvercompare func in 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
1 parent b311592 commit fe2ffb2

File tree

10 files changed

+153
-12
lines changed

10 files changed

+153
-12
lines changed

api/helm-app/gRPC/applist.pb.go

Lines changed: 24 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/helm-app/gRPC/applist.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ message UpgradeReleaseRequest {
258258
int32 historyMax = 3;
259259
ChartContent chartContent = 4;
260260
bool RunInCtx = 5;
261+
string K8sVersion = 6;
261262
}
262263

263264
message UpgradeReleaseResponse {
@@ -317,6 +318,7 @@ message HelmInstallCustomRequest {
317318
ChartContent chartContent = 2;
318319
ReleaseIdentifier releaseIdentifier = 3;
319320
bool RunInCtx = 4;
321+
string K8sVersion = 5;
320322
}
321323

322324
message HelmInstallCustomResponse {

api/helm-app/service/HelmAppService.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,11 @@ type HelmAppService interface {
5757
GetDesiredManifest(ctx context.Context, app *AppIdentifier, resource *openapi.ResourceIdentifier) (*openapi.DesiredManifestResponse, error)
5858
DeleteApplication(ctx context.Context, app *AppIdentifier) (*openapi.UninstallReleaseResponse, error)
5959
DeleteDBLinkedHelmApplication(ctx context.Context, app *AppIdentifier, useId int32) (*openapi.UninstallReleaseResponse, error)
60+
// UpdateApplication is a wrapper over helmAppClient.UpdateApplication, sends update request to kubelink for external chart store apps
6061
UpdateApplication(ctx context.Context, app *AppIdentifier, request *bean.UpdateApplicationRequestDto) (*openapi.UpdateReleaseResponse, error)
6162
GetDeploymentDetail(ctx context.Context, app *AppIdentifier, version int32) (*openapi.HelmAppDeploymentManifestDetail, error)
6263
InstallRelease(ctx context.Context, clusterId int, installReleaseRequest *gRPC.InstallReleaseRequest) (*gRPC.InstallReleaseResponse, error)
64+
// UpdateApplicationWithChartInfo is a wrapper over helmAppClient.UpdateApplicationWithChartInfo sends update request to kubelink for helm chart store apps
6365
UpdateApplicationWithChartInfo(ctx context.Context, clusterId int, request *bean.UpdateApplicationWithChartInfoRequestDto) (*openapi.UpdateReleaseResponse, error)
6466
IsReleaseInstalled(ctx context.Context, app *AppIdentifier) (bool, error)
6567
RollbackRelease(ctx context.Context, app *AppIdentifier, version int32) (bool, error)

env_gen.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,9 @@
196196
| REQ_CI_MEM | 3G | |
197197
| RESOURCE_LIST_FOR_REPLICAS | Deployment,Rollout,StatefulSet,ReplicaSet | |
198198
| RESOURCE_LIST_FOR_REPLICAS_BATCH_SIZE | 5 | |
199-
| REVISION_HISTORY_LIMIT_DEVTRON_APP | 0 | |
199+
| REVISION_HISTORY_LIMIT_DEVTRON_APP | 1 | |
200200
| REVISION_HISTORY_LIMIT_EXTERNAL_HELM_APP | 0 | |
201-
| REVISION_HISTORY_LIMIT_HELM_APP | 0 | |
201+
| REVISION_HISTORY_LIMIT_HELM_APP | 1 | |
202202
| RUNTIME_CONFIG_LOCAL_DEV | false | |
203203
| RUN_HELM_INSTALL_IN_ASYNC_MODE_HELM_APPS | false | |
204204
| SCOPED_VARIABLE_ENABLED | false | |

pkg/deployment/trigger/devtronApps/TriggerService.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
4343
"github.com/devtron-labs/devtron/pkg/eventProcessor/out"
4444
"github.com/devtron-labs/devtron/pkg/imageDigestPolicy"
45+
k8s2 "github.com/devtron-labs/devtron/pkg/k8s"
4546
"github.com/devtron-labs/devtron/pkg/pipeline"
4647
bean8 "github.com/devtron-labs/devtron/pkg/pipeline/bean"
4748
"github.com/devtron-labs/devtron/pkg/pipeline/history"
@@ -64,6 +65,7 @@ import (
6465
"k8s.io/helm/pkg/proto/hapi/chart"
6566
"net/http"
6667
"path"
68+
"regexp"
6769
"strconv"
6870
"strings"
6971
"time"
@@ -137,6 +139,7 @@ type TriggerServiceImpl struct {
137139
ciPipelineRepository pipelineConfig.CiPipelineRepository
138140
appWorkflowRepository appWorkflow.AppWorkflowRepository
139141
dockerArtifactStoreRepository repository4.DockerArtifactStoreRepository
142+
K8sUtil *util5.K8sServiceImpl
140143
}
141144

142145
func NewTriggerServiceImpl(logger *zap.SugaredLogger, cdWorkflowCommonService cd.CdWorkflowCommonService,
@@ -186,7 +189,8 @@ func NewTriggerServiceImpl(logger *zap.SugaredLogger, cdWorkflowCommonService cd
186189
ciPipelineRepository pipelineConfig.CiPipelineRepository,
187190
appWorkflowRepository appWorkflow.AppWorkflowRepository,
188191
dockerArtifactStoreRepository repository4.DockerArtifactStoreRepository,
189-
imageScanService security2.ImageScanService) (*TriggerServiceImpl, error) {
192+
imageScanService security2.ImageScanService,
193+
K8sUtil *util5.K8sServiceImpl) (*TriggerServiceImpl, error) {
190194
impl := &TriggerServiceImpl{
191195
logger: logger,
192196
cdWorkflowCommonService: cdWorkflowCommonService,
@@ -237,6 +241,7 @@ func NewTriggerServiceImpl(logger *zap.SugaredLogger, cdWorkflowCommonService cd
237241
appWorkflowRepository: appWorkflowRepository,
238242
dockerArtifactStoreRepository: dockerArtifactStoreRepository,
239243
imageScanService: imageScanService,
244+
K8sUtil: K8sUtil,
240245
}
241246
config, err := types.GetCdConfig()
242247
if err != nil {
@@ -877,9 +882,23 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean
877882
Name: pipeline.App.AppName,
878883
Version: envOverride.Chart.ChartVersion,
879884
}
880-
referenceTemplatePath := path.Join(bean5.RefChartDirPath, envOverride.Chart.ReferenceTemplate)
885+
referenceTemplate := envOverride.Chart.ReferenceTemplate
886+
referenceTemplatePath := path.Join(bean5.RefChartDirPath, referenceTemplate)
881887

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

949971
} else {
950972

951-
helmResponse, err := impl.helmInstallReleaseWithCustomChart(ctx, releaseIdentifier, referenceChartByte, mergeAndSave)
973+
helmResponse, err := impl.helmInstallReleaseWithCustomChart(ctx, releaseIdentifier, referenceChartByte, mergeAndSave, sanitizedK8sVersion)
952974

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

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

11591181
helmInstallRequest := gRPC.HelmInstallCustomRequest{
11601182
ValuesYaml: valuesYaml,
11611183
ChartContent: &gRPC.ChartContent{Content: referenceChartByte},
11621184
ReleaseIdentifier: releaseIdentifier,
11631185
}
1186+
if len(k8sServerVersion) > 0 {
1187+
helmInstallRequest.K8SVersion = k8sServerVersion
1188+
}
11641189
if impl.isDevtronAsyncInstallModeEnabled(bean.Helm) {
11651190
helmInstallRequest.RunInCtx = true
11661191
}

pkg/deployment/trigger/devtronApps/bean/bean.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,7 @@ type VulnerabilityCheckRequest struct {
6060
ImageDigest string
6161
CdPipeline *pipelineConfig.Pipeline
6262
}
63+
64+
const (
65+
CronJobChartRegexExpression = "cronjob-chart_1-(2|3|4|5)-0"
66+
)

pkg/generateManifest/DeployementTemplateService.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"github.com/devtron-labs/devtron/pkg/chart"
1616
repository3 "github.com/devtron-labs/devtron/pkg/cluster/repository"
1717
"github.com/devtron-labs/devtron/pkg/deployment/manifest/deploymentTemplate/chartRef"
18+
bean2 "github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/bean"
19+
k8s2 "github.com/devtron-labs/devtron/pkg/k8s"
1820
"github.com/devtron-labs/devtron/pkg/pipeline"
1921
"github.com/devtron-labs/devtron/pkg/pipeline/history"
2022
"github.com/devtron-labs/devtron/pkg/resourceQualifiers"
@@ -25,6 +27,7 @@ import (
2527
"go.uber.org/zap"
2628
"net/http"
2729
"os"
30+
"regexp"
2831
"strconv"
2932
"time"
3033
)
@@ -336,11 +339,21 @@ func (impl DeploymentTemplateServiceImpl) GenerateManifest(ctx context.Context,
336339
impl.Logger.Errorw("exception caught in getting k8sServerVersion", "err", err)
337340
return nil, err
338341
}
342+
343+
sanitizedK8sVersion := k8sServerVersion.String()
344+
//handle specific case for all cronjob charts from cronjob-chart_1-2-0 to cronjob-chart_1-5-0 where semverCompare
345+
//comparison func has wrong api version mentioned, so for already installed charts via these charts that comparison
346+
//is always false, handles the gh issue:- https://github.com/devtron-labs/devtron/issues/4860
347+
cronJobChartRegex := regexp.MustCompile(bean2.CronJobChartRegexExpression)
348+
if cronJobChartRegex.MatchString(template) {
349+
sanitizedK8sVersion = k8s2.StripPrereleaseFromK8sVersion(sanitizedK8sVersion)
350+
}
351+
339352
installReleaseRequest := &gRPC.InstallReleaseRequest{
340353
ChartName: template,
341354
ChartVersion: version,
342355
ValuesYaml: valuesYaml,
343-
K8SVersion: k8sServerVersion.String(),
356+
K8SVersion: sanitizedK8sVersion,
344357
ChartRepository: ChartRepository,
345358
ReleaseIdentifier: ReleaseIdentifier,
346359
ChartContent: &gRPC.ChartContent{

pkg/k8s/helper.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package k8s
22

33
import (
4+
"fmt"
5+
"github.com/Masterminds/semver"
46
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
57
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"strings"
69
)
710

811
func IsResourceNotFoundErr(err error) bool {
@@ -11,3 +14,18 @@ func IsResourceNotFoundErr(err error) bool {
1114
}
1215
return false
1316
}
17+
18+
// StripPrereleaseFromK8sVersion takes in k8sVersion and stripe pre-release from semver version and return sanitized k8sVersion
19+
// or error if invalid version provided, e.g. if k8sVersion = "1.25.16-eks-b9c9ed7", then it returns "1.25.16".
20+
func StripPrereleaseFromK8sVersion(k8sVersion string) string {
21+
version, err := semver.NewVersion(k8sVersion)
22+
if err != nil {
23+
fmt.Printf("error in stripping pre-release from k8sServerVersion due to invalid k8sServerVersion:= %s, err:= %v", k8sVersion, err)
24+
return k8sVersion
25+
}
26+
if len(version.Prerelease()) > 0 {
27+
stringToReplace := "-" + version.Prerelease()
28+
k8sVersion = strings.Replace(k8sVersion, stringToReplace, "", 1)
29+
}
30+
return k8sVersion
31+
}

pkg/k8s/helper_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package k8s
2+
3+
import "testing"
4+
5+
// not removing metadata from k8s version if exists, we only eliminate pre-release from k8s version
6+
const (
7+
K8sVersionWithPreRelease = "v1.25.16-eks-b9c9ed7"
8+
K8sVersionWithPreReleaseAndMetadata = "v1.25.16-eks-b9c9ed7+acj23-as"
9+
K8sVersionWithMetadata = "v1.25.16+acj23-as"
10+
K8sVersionWithoutPreReleaseAndMetadata = "v1.25.16"
11+
InvalidK8sVersion = ""
12+
)
13+
14+
func TestStripPrereleaseFromK8sVersion(t *testing.T) {
15+
type args struct {
16+
k8sVersion string
17+
}
18+
tests := []struct {
19+
name string
20+
args args
21+
want string
22+
}{
23+
{
24+
name: "Test1_K8sVersionWithPreRelease",
25+
args: args{k8sVersion: K8sVersionWithPreRelease},
26+
want: K8sVersionWithoutPreReleaseAndMetadata,
27+
},
28+
{
29+
name: "Test2_K8sVersionWithPreReleaseAndMetadata",
30+
args: args{k8sVersion: K8sVersionWithPreReleaseAndMetadata},
31+
want: K8sVersionWithMetadata,
32+
},
33+
{
34+
name: "Test3_K8sVersionWithMetadata",
35+
args: args{k8sVersion: K8sVersionWithMetadata},
36+
want: K8sVersionWithMetadata,
37+
},
38+
{
39+
name: "Test4_K8sVersionWithoutPrereleaseAndMetadata",
40+
args: args{k8sVersion: K8sVersionWithoutPreReleaseAndMetadata},
41+
want: K8sVersionWithoutPreReleaseAndMetadata,
42+
},
43+
{
44+
name: "Test5_EmptyK8sVersion",
45+
args: args{k8sVersion: InvalidK8sVersion},
46+
want: InvalidK8sVersion,
47+
},
48+
}
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
got := StripPrereleaseFromK8sVersion(tt.args.k8sVersion)
52+
if got != tt.want {
53+
t.Errorf("StripPrereleaseFromK8sVersion() got = %v, want %v", got, tt.want)
54+
}
55+
})
56+
}
57+
}

0 commit comments

Comments
 (0)