Skip to content

Commit

Permalink
Merge branch 'main' into feat/refactor-template-lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
@c-pius
c-pius authored Jan 23, 2025
2 parents 6129b2d + f025813 commit 625f849
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 41 deletions.
50 changes: 24 additions & 26 deletions api/v1beta2/moduletemplate_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,39 +223,27 @@ func (m *ModuleTemplate) IsInternal() bool {
return false
}

var ErrInvalidVersion = errors.New("can't find valid semantic version")

// getVersionLegacy() returns the version of the ModuleTemplate from the annotation on the object.
// Remove once shared.ModuleVersionAnnotation is removed.
func (m *ModuleTemplate) getVersionLegacy() (string, error) {
if m.Annotations != nil {
moduleVersion, found := m.Annotations[shared.ModuleVersionAnnotation]
if found {
return moduleVersion, nil
}
// https://github.com/kyma-project/lifecycle-manager/issues/2096
// Refactor this function to drop the label fallback after the migration to the new ModuleTemplate format is completed.
func (m *ModuleTemplate) GetVersion() string {
version := m.Spec.Version
if version == "" {
version = m.Annotations[shared.ModuleVersionAnnotation]
}
return "", ErrInvalidVersion
return version
}

// GetVersion returns the declared version of the ModuleTemplate from it's Spec.
func (m *ModuleTemplate) GetVersion() (*semver.Version, error) {
var versionValue string
var err error

if m.Spec.Version == "" {
versionValue, err = m.getVersionLegacy()
if err != nil {
return nil, err
}
} else {
versionValue = m.Spec.Version
}
var ErrInvalidVersion = errors.New("can't find valid semantic version")

version, err := semver.NewVersion(versionValue)
// GetSemanticVersion returns the declared version of the ModuleTemplate as semantic version.
func (m *ModuleTemplate) GetSemanticVersion() (*semver.Version, error) {
version := m.GetVersion()

semanticVersion, err := semver.NewVersion(version)
if err != nil {
return nil, fmt.Errorf("%w: %s", ErrInvalidVersion, err.Error())
}
return version, nil
return semanticVersion, nil
}

// https://github.com/kyma-project/lifecycle-manager/issues/2096
Expand All @@ -279,3 +267,13 @@ func (m *ModuleTemplate) HasSyncDisabled() bool {
}
return false
}

// https://github.com/kyma-project/lifecycle-manager/issues/2096
// Refactor this function to drop the label fallback after the migration to the new ModuleTemplate format is completed.
func (m *ModuleTemplate) GetModuleName() string {
moduleName := m.Spec.ModuleName
if moduleName == "" {
moduleName = m.Labels[shared.ModuleName]
}
return moduleName
}
131 changes: 120 additions & 11 deletions api/v1beta2/moduletemplate_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/api/v1beta2"
)

func Test_GetVersion(t *testing.T) {
func Test_GetSemanticVersion(t *testing.T) {
const testVersion = "1.0.1"
const otherVersion = "0.0.1"
tests := []struct {
Expand All @@ -20,7 +21,7 @@ func Test_GetVersion(t *testing.T) {
expectedErr string
}{
{
name: "Test GetVersion() by annotation (legacy)",
name: "Test GetSemanticVersion() by annotation (legacy)",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Annotations: map[string]string{
Expand All @@ -31,7 +32,7 @@ func Test_GetVersion(t *testing.T) {
expectedVersion: testVersion,
},
{
name: "Test GetVersion() by explicit version in Spec",
name: "Test GetSemanticVersion() by explicit version in Spec",
m: &v1beta2.ModuleTemplate{
Spec: v1beta2.ModuleTemplateSpec{
Version: testVersion,
Expand All @@ -40,7 +41,7 @@ func Test_GetVersion(t *testing.T) {
expectedVersion: testVersion,
},
{
name: "Test GetVersion() with both version in Spec and annotation",
name: "Test GetSemanticVersion() with both version in Spec and annotation",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Annotations: map[string]string{
Expand Down Expand Up @@ -75,32 +76,140 @@ func Test_GetVersion(t *testing.T) {
}
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
actualVersion, err := testCase.m.GetVersion()
actualVersion, err := testCase.m.GetSemanticVersion()
if err != nil {
if actualVersion != nil {
t.Errorf("GetVersion(): Returned version should be nil when error is not nil")
t.Errorf("GetSemanticVersion(): Returned version should be nil when error is not nil")
}
if testCase.expectedErr == "" {
t.Errorf("GetVersion(): Unexpected error: %v", err)
t.Errorf("GetSemanticVersion(): Unexpected error: %v", err)
}
if !strings.Contains(err.Error(), testCase.expectedErr) {
t.Errorf("GetVersion(): Actual error = %v, expected error: %v", err, testCase.expectedErr)
t.Errorf("GetSemanticVersion(): Actual error = %v, expected error: %v", err, testCase.expectedErr)
}
return
}

if actualVersion == nil {
t.Errorf("GetVersion(): Returned version should not be nil when error is nil")
t.Errorf("GetSemanticVersion(): Returned version should not be nil when error is nil")
}

if testCase.expectedVersion == "" {
t.Errorf("GetVersion(): Expected version is empty but non-nil version is returned")
t.Errorf("GetSemanticVersion(): Expected version is empty but non-nil version is returned")
}

if actualVersion != nil && actualVersion.String() != testCase.expectedVersion {
t.Errorf("GetVersion(): actual version = %v, expected version: %v", actualVersion.String(),
t.Errorf("GetSemanticVersion(): actual version = %v, expected version: %v", actualVersion.String(),
testCase.expectedVersion)
}
})
}
}

//nolint:dupl // similar but not duplicate
func Test_GetVersion(t *testing.T) {
tests := []struct {
name string
m *v1beta2.ModuleTemplate
expectedVersion string
}{
{
name: "Test GetVersion() by annotation (legacy)",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Annotations: map[string]string{
shared.ModuleVersionAnnotation: "1.0.0-annotated",
},
},
Spec: v1beta2.ModuleTemplateSpec{},
},
expectedVersion: "1.0.0-annotated",
},
{
name: "Test GetVersion() by spec.version",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Annotations: map[string]string{},
},
Spec: v1beta2.ModuleTemplateSpec{
Version: "2.0.0-spec",
},
},
expectedVersion: "2.0.0-spec",
},
{
name: "Test GetVersion() spec.moduleName has priority over annotation",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Annotations: map[string]string{
shared.ModuleVersionAnnotation: "1.0.0-annotated",
},
},
Spec: v1beta2.ModuleTemplateSpec{
Version: "2.0.0-spec",
},
},
expectedVersion: "2.0.0-spec",
},
}
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
actualVersion := testCase.m.GetVersion()
assert.Equal(t, testCase.expectedVersion, actualVersion)
})
}
}

//nolint:dupl // similar but not duplicate
func Test_GetModuleName(t *testing.T) {
tests := []struct {
name string
m *v1beta2.ModuleTemplate
expectedName string
}{
{
name: "Test GetModuleName() by label",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Labels: map[string]string{
shared.ModuleName: "labelled-module",
},
},
Spec: v1beta2.ModuleTemplateSpec{},
},
expectedName: "labelled-module",
},
{
name: "Test GetModuleName() by spec.moduleName",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Labels: map[string]string{},
},
Spec: v1beta2.ModuleTemplateSpec{
ModuleName: "spec-module",
},
},
expectedName: "spec-module",
},
{
name: "Test GetModuleName() spec.moduleName has priority over label",
m: &v1beta2.ModuleTemplate{
ObjectMeta: apimetav1.ObjectMeta{
Labels: map[string]string{
shared.ModuleName: "labelled-module",
},
},
Spec: v1beta2.ModuleTemplateSpec{
ModuleName: "spec-module",
},
},
expectedName: "spec-module",
},
}
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
actualName := testCase.m.GetModuleName()
assert.Equal(t, testCase.expectedName, actualName)
})
}
}
2 changes: 1 addition & 1 deletion internal/descriptor/cache/key.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
type DescriptorKey string

func GenerateDescriptorKey(template *v1beta2.ModuleTemplate) DescriptorKey {
version, err := template.GetVersion()
version, err := template.GetSemanticVersion()
if err == nil {
return DescriptorKey(fmt.Sprintf("%s:%s:%d:%s", template.Name, template.Spec.Channel, template.Generation,
version))
Expand Down
5 changes: 2 additions & 3 deletions internal/maintenancewindows/maintenance_window.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,13 @@ func (MaintenanceWindow) IsRequired(moduleTemplate *v1beta2.ModuleTemplate, kyma
}

// module not installed yet => no need for maintenance window
moduleStatus := kyma.Status.GetModuleStatus(moduleTemplate.Spec.ModuleName)
moduleStatus := kyma.Status.GetModuleStatus(moduleTemplate.GetModuleName())
if moduleStatus == nil {
return false
}

// module already installed in this version => no need for maintenance window
installedVersion := moduleStatus.Version
return installedVersion != moduleTemplate.Spec.Version
return moduleStatus.Version != moduleTemplate.GetVersion()
}

// IsActive determines if a maintenance window is currently active.
Expand Down

0 comments on commit 625f849

Please sign in to comment.