Skip to content

Commit

Permalink
fix: Fix module version downgrade bug with the new ModuleTemplate (#2043
Browse files Browse the repository at this point in the history 
)

* fix module downgrade issue with modulereleasemeta

* adjust unit test

* Fix E2E test (use the fast channel for enabling the module first)

* Remove channel from ModuleTemplates used in version downgrade E2E test

* Remove channel check for version downgrade

* Remove channels in ModuleTemplates used for E2E with ModuleReleaseMeta

* Add manifest is ready state before upgrading version

* Add expected message for the kyma module in kyma status

* Remove left channel condition

* Fix unit test

* Fix unit test coverage

* fix error message

* Add test case message
  • Loading branch information
@nesmabadr
nesmabadr authored Nov 22, 2024
1 parent 5233c27 commit dd8ba97
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 22 deletions.
7 changes: 7 additions & 0 deletions .github/actions/deploy-template-operator-with-modulereleasemeta/action.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ runs:
sed -i 's/template-operator-regular/template-operator-1.1.1-e2e-test/g' tests/e2e/moduletemplate/moduletemplate_template_operator_v1_regular.yaml
sed -i 's/template-operator-regular/template-operator-2.4.2-e2e-test/g' tests/e2e/moduletemplate/moduletemplate_template_operator_v2_regular_new_version.yaml
sed -i '/^ channel: fast$/d' tests/e2e/moduletemplate/moduletemplate_template_operator_v2_fast.yaml
sed -i '/^ channel: regular$/d' tests/e2e/moduletemplate/moduletemplate_template_operator_v1_regular.yaml
sed -i '/^ channel: regular$/d' tests/e2e/moduletemplate/moduletemplate_template_operator_v2_regular_new_version.yaml
kubectl apply -f tests/e2e/moduletemplate/moduletemplate_template_operator_v2_fast.yaml
kubectl apply -f tests/e2e/moduletemplate/moduletemplate_template_operator_v1_regular.yaml
Expand Down Expand Up @@ -82,6 +86,7 @@ runs:
shell: bash
run: |
sed -i 's/template-operator-fast/template-operator-2.4.2-e2e-test/g' tests/e2e/moduletemplate/moduletemplate_template_operator_v2_fast.yaml
sed -i '/^ channel: fast$/d' tests/e2e/moduletemplate/moduletemplate_template_operator_v2_fast.yaml
kubectl apply -f tests/e2e/moduletemplate/moduletemplate_template_operator_v2_fast.yaml
cat <<EOF > mrm.yaml
Expand Down Expand Up @@ -271,6 +276,8 @@ runs:
shell: bash
run: |
sed -i 's/template-operator-regular/template-operator-1.0.0-new-ocm-format/g' tests/e2e/moduletemplate/moduletemplate_template_operator_regular_new_ocm.yaml
sed -i '/^ channel: regular/d' tests/e2e/moduletemplate/moduletemplate_template_operator_regular_new_ocm.yaml
kubectl apply -f tests/e2e/moduletemplate/moduletemplate_template_operator_regular_new_ocm.yaml
cat <<EOF > mrm.yaml
apiVersion: operator.kyma-project.io/v1beta2
Expand Down
2 changes: 1 addition & 1 deletion pkg/module/sync/runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func generateModuleStatus(module *common.Module, existStatus *v1beta2.ModuleStat
Name: module.ModuleName,
FQDN: module.FQDN,
State: manifestObject.Status.State,
Channel: module.Template.Spec.Channel,
Channel: module.Template.DesiredChannel,
Version: manifestObject.Spec.Version,
Manifest: &v1beta2.TrackingObject{
PartialMeta: v1beta2.PartialMeta{
Expand Down
13 changes: 5 additions & 8 deletions pkg/templatelookup/regular.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2.
templateInfo.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg)
continue
}
markInvalidChannelSkewUpdate(ctx, &templateInfo, moduleStatus, descriptor.Version)
markInvalidSkewUpdate(ctx, &templateInfo, moduleStatus, descriptor.Version)
}
}
templates[module.Name] = &templateInfo
Expand Down Expand Up @@ -231,8 +231,8 @@ func moduleMatch(moduleStatus *v1beta2.ModuleStatus, moduleName string) bool {
return moduleStatus.Name == moduleName
}

// markInvalidChannelSkewUpdate verifies if the given ModuleTemplate is invalid for update when channel switch is detected.
func markInvalidChannelSkewUpdate(ctx context.Context, moduleTemplateInfo *ModuleTemplateInfo,
// markInvalidSkewUpdate verifies if the given ModuleTemplate is invalid for update.
func markInvalidSkewUpdate(ctx context.Context, moduleTemplateInfo *ModuleTemplateInfo,
moduleStatus *v1beta2.ModuleStatus, templateVersion string,
) {
if moduleStatus.Template == nil {
Expand All @@ -247,13 +247,10 @@ func markInvalidChannelSkewUpdate(ctx context.Context, moduleTemplateInfo *Modul
"template", moduleTemplateInfo.Name,
"newTemplateGeneration", moduleTemplateInfo.GetGeneration(),
"previousTemplateGeneration", moduleStatus.Template.GetGeneration(),
"newTemplateChannel", moduleTemplateInfo.Spec.Channel,
"newTemplateChannel", moduleTemplateInfo.DesiredChannel,
"previousTemplateChannel", moduleStatus.Channel,
)

if moduleTemplateInfo.Spec.Channel == moduleStatus.Channel && moduleTemplateInfo.Spec.Channel != string(shared.NoneChannel) {
return
}
checkLog.Info("outdated ModuleTemplate: channel skew")

versionInTemplate, err := semver.NewVersion(templateVersion)
Expand Down Expand Up @@ -287,7 +284,7 @@ func markInvalidChannelSkewUpdate(ctx context.Context, moduleTemplateInfo *Modul
if !v1beta2.IsValidVersionChange(versionInTemplate, versionInStatus) {
msg := fmt.Sprintf("ignore channel skew (from %s to %s), "+
"as a higher version (%s) of the module was previously installed",
moduleStatus.Channel, moduleTemplateInfo.Spec.Channel, versionInStatus.String())
moduleStatus.Channel, moduleTemplateInfo.DesiredChannel, versionInStatus.String())
checkLog.Info(msg)
moduleTemplateInfo.Err = fmt.Errorf("%w: %s", ErrTemplateUpdateNotAllowed, msg)
}
Expand Down
34 changes: 22 additions & 12 deletions pkg/templatelookup/regular_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchModuleChannel(t *testing.T
},
},
}).Build(),
availableModuleTemplate: generateModuleTemplateListWithModule(testModule.Name, testModule.Channel,
availableModuleTemplate: generateModuleTemplateListWithModule(testModule.Name, "",
version2),
availableModuleReleaseMeta: generateModuleReleaseMetaList(testModule.Name,
[]v1beta2.ChannelVersionAssignment{
Expand All @@ -271,15 +271,14 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchModuleChannel(t *testing.T
WithEnabledModule(testModule).
WithModuleStatus(v1beta2.ModuleStatus{
Name: testModule.Name,
Channel: v1beta2.DefaultChannel,
Version: version2,
Template: &v1beta2.TrackingObject{
PartialMeta: v1beta2.PartialMeta{
Generation: 1,
},
},
}).Build(),
availableModuleTemplate: generateModuleTemplateListWithModule(testModule.Name, testModule.Channel,
availableModuleTemplate: generateModuleTemplateListWithModule(testModule.Name, "",
version1),
availableModuleReleaseMeta: generateModuleReleaseMetaList(testModule.Name,
[]v1beta2.ChannelVersionAssignment{
Expand Down Expand Up @@ -780,7 +779,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
Err: nil,
ModuleTemplate: builder.NewModuleTemplateBuilder().
WithModuleName(testModule.Name).
WithChannel(testModule.Channel).
WithChannel("").
Build(),
},
},
Expand All @@ -797,6 +796,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
Generation: 1,
},
},
Version: "1.0.0",
}).Build(),
mrmExist: false,
want: templatelookup.ModuleTemplatesByModuleName{
Expand All @@ -822,6 +822,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
Generation: 1,
},
},
Version: "1.0.0",
}).Build(),
mrmExist: true,
want: templatelookup.ModuleTemplatesByModuleName{
Expand All @@ -830,7 +831,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
Err: nil,
ModuleTemplate: builder.NewModuleTemplateBuilder().
WithModuleName(testModule.Name).
WithChannel(testModule.Channel).
WithChannel("").
Build(),
},
},
Expand All @@ -841,19 +842,26 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
givenTemplateList := &v1beta2.ModuleTemplateList{}
moduleReleaseMetas := v1beta2.ModuleReleaseMetaList{}
for _, module := range templatelookup.FindAvailableModules(testCase.kyma) {
givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder().
WithName(fmt.Sprintf("%s-%s", module.Name, testModule.Version)).
WithModuleName(module.Name).
WithLabelModuleName(module.Name).
WithChannel(module.Channel).
WithOCM(compdescv2.SchemaVersion).Build())
if testCase.mrmExist {
givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder().
WithName(fmt.Sprintf("%s-%s", module.Name, testModule.Version)).
WithModuleName(module.Name).
WithLabelModuleName(module.Name).
WithChannel("").
WithOCM(compdescv2.SchemaVersion).Build())
moduleReleaseMetas.Items = append(moduleReleaseMetas.Items,
*builder.NewModuleReleaseMetaBuilder().
WithModuleName(module.Name).
WithModuleChannelAndVersions([]v1beta2.ChannelVersionAssignment{
{Channel: module.Channel, Version: testModule.Version},
}).Build())
} else {
givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder().
WithName(fmt.Sprintf("%s-%s", module.Name, testModule.Version)).
WithModuleName(module.Name).
WithLabelModuleName(module.Name).
WithChannel(module.Channel).
WithOCM(compdescv2.SchemaVersion).Build())
}
}
lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList,
Expand All @@ -866,7 +874,9 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
assert.True(t, ok)
assert.Equal(t, wantModule.DesiredChannel, module.DesiredChannel)
require.ErrorIs(t, module.Err, wantModule.Err)
assert.Equal(t, wantModule.ModuleTemplate.Spec.Channel, module.ModuleTemplate.Spec.Channel)
if !testCase.mrmExist {
assert.Equal(t, wantModule.ModuleTemplate.Spec.Channel, module.ModuleTemplate.Spec.Channel)
}
}
})
}
Expand Down
21 changes: 21 additions & 0 deletions tests/e2e/module_upgrade_channel_switch_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ var _ = Describe("Module Upgrade By Channel Switch", Ordered, func() {
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateReady).
Should(Succeed())

By("And Manifest CR is in \"Ready\" State")
Eventually(CheckManifestIsInState).
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, kcpClient, shared.StateReady).
Should(Succeed())
})

It("When upgrade version by switch Channel", func() {
Expand Down Expand Up @@ -73,6 +79,12 @@ var _ = Describe("Module Upgrade By Channel Switch", Ordered, func() {
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateReady).
Should(Succeed())

By("And Manifest CR is in \"Ready\" State")
Eventually(CheckManifestIsInState).
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, kcpClient, shared.StateReady).
Should(Succeed())
})

It("When downgrade version by switch Channel", func() {
Expand All @@ -83,6 +95,8 @@ var _ = Describe("Module Upgrade By Channel Switch", Ordered, func() {
})

It("Then Module stay in newer version", func() {
expectedErrorMessage := "module template update not allowed: ignore channel skew (from fast to regular), as a higher version (2.4.2-e2e-test) of the module was previously installed"

Eventually(ModuleCRExists).
WithContext(ctx).
WithArguments(skrClient, moduleCR).
Expand All @@ -105,6 +119,13 @@ var _ = Describe("Module Upgrade By Channel Switch", Ordered, func() {
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateWarning).
Should(Succeed())

By("And the Module Status has correct error message", func() {
Eventually(ModuleMessageInKymaStatusIsCorrect).
WithContext(ctx).
WithArguments(kcpClient, kyma.GetName(), kyma.GetNamespace(), module.Name, expectedErrorMessage).
Should(Succeed())
})
})
})
})
2 changes: 1 addition & 1 deletion unit-test-coverage.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ packages:
internal/pkg/resources: 91.7
internal/remote: 13.2
internal/util/collections: 86
pkg/templatelookup: 77.3
pkg/templatelookup: 77.1

0 comments on commit dd8ba97

Please sign in to comment.