Skip to content

Commit

Permalink
Revert "Merge branch 'main' into intro-test-managed-resources"
Browse files Browse the repository at this point in the history
This reverts commit 854b8b4, reversing
changes made to be4f556.
  • Loading branch information
lindnerby committed Jul 9, 2024
1 parent 737af7c commit 9d639f3
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 210 deletions.
2 changes: 1 addition & 1 deletion .github/actions/get-configuration/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ runs:
id: define-variables
shell: bash
run: |
echo "k8s_version=${{ github.event.inputs.k8s_version || '1.29.6' }}" >> $GITHUB_OUTPUT
echo "k8s_version=${{ github.event.inputs.k8s_version || '1.28.7' }}" >> $GITHUB_OUTPUT
echo "istio_version=1.20.3" >> $GITHUB_OUTPUT
echo "k3d_version=5.6.0" >> $GITHUB_OUTPUT
echo "cert_manager_version=1.15.0" >> $GITHUB_OUTPUT
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
- uses: ./.github/actions/wait-for-image-build
with:
token: ${{ secrets.GITHUB_TOKEN }}
statusName: ${{ (github.event_name == 'pull_request') && 'pull-lifecycle-mgr-build' || 'main-lifecycle-mgr-build' }}
statusName: pull-lifecycle-mgr-build
e2e-integration:
name: E2E
needs: wait-for-image-build
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
env:
LIFECYCLE_MANAGER: ${{ github.repository }}
K3D_VERSION: v5.6.0
K8S_VERSION: v1.29.6
K8S_VERSION: v1.28.7
KUSTOMIZE_VERSION: 5.3.0
ISTIO_VERSION: 1.20.3
GOSUMDB: off
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Build the manager binary
FROM golang:1.22.5-alpine as builder
FROM golang:1.22.4-alpine as builder

WORKDIR /lifecycle-manager
# Copy the Go Modules manifests
Expand Down

This file was deleted.

11 changes: 0 additions & 11 deletions docs/technical-reference/api/manifest-cr.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,6 @@

The [Manifest custom resource (CR)](../../../api/v1beta2/manifest_types.go) is our internal representation of what results from the resolution of a ModuleTemplate CR in the context of a single cluster represented by a Kyma CR. Thus, a lot of configuration elements are similar or entirely equivalent to the data layer found in a ModuleTemplate CR.

## Patching

The [Runner](../../../pkg/module/sync/runner.go) is responsible for creating and updating Manifest CRs using Server Side Apply (SSA). An update is only performed when one of the following conditions is met:

1. The Manifest CR version differs from the Kyma CR's module status version.
2. The Manifest CR channel differs from the Kyma CR's module status channel.
3. The Manifest CR state differs from the Kyma CR's module status state.

>[!NOTE]
>The module status is not present in the Kyma CR for mandatory modules, hence their Manifest CR is updated using SSA in every reconcile loop.
## Configuration

### **.spec.remote**
Expand Down
45 changes: 12 additions & 33 deletions pkg/module/sync/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (r *Runner) ReconcileManifests(ctx context.Context, kyma *v1beta2.Kyma,
results <- nil
return
}
if err := r.updateManifest(ctx, kyma, module); err != nil {
if err := r.updateManifests(ctx, kyma, module); err != nil {
results <- fmt.Errorf("could not update module %s: %w", module.GetName(), err)
return
}
Expand Down Expand Up @@ -95,7 +95,7 @@ func (r *Runner) getModule(ctx context.Context, module client.Object) error {
return nil
}

func (r *Runner) updateManifest(ctx context.Context, kyma *v1beta2.Kyma,
func (r *Runner) updateManifests(ctx context.Context, kyma *v1beta2.Kyma,
module *common.Module,
) error {
if err := r.setupModule(module, kyma); err != nil {
Expand All @@ -110,34 +110,17 @@ func (r *Runner) updateManifest(ctx context.Context, kyma *v1beta2.Kyma,
return commonerrs.ErrTypeAssert
}

moduleStatus := kyma.GetModuleStatusMap()[module.ModuleName]
if err := r.doUpdateWithStrategy(ctx, kyma.Labels[shared.ManagedBy], module.Enabled,
manifestObj, moduleStatus); err != nil {
manifestObj); err != nil {
return err
}
module.Manifest = manifestObj
return nil
}

func (r *Runner) doUpdateWithStrategy(ctx context.Context, owner string, isEnabledModule bool,
manifestObj *v1beta2.Manifest, kymaModuleStatus *v1beta2.ModuleStatus,
manifestObj *v1beta2.Manifest,
) error {
manifestInCluster := &v1beta2.Manifest{}
if err := r.Get(ctx, client.ObjectKey{
Namespace: manifestObj.GetNamespace(),
Name: manifestObj.GetName(),
}, manifestInCluster); err != nil {
if !util.IsNotFound(err) {
return fmt.Errorf("error get manifest %s: %w", client.ObjectKeyFromObject(manifestObj), err)
}
manifestInCluster = nil
}

if !NeedToUpdate(manifestInCluster, manifestObj, kymaModuleStatus) {
// Point to the current state from the cluster for the outside sync of the manifest
*manifestObj = *manifestInCluster
return nil
}
if isEnabledModule {
return r.patchManifest(ctx, owner, manifestObj)
}
Expand All @@ -161,27 +144,23 @@ func (r *Runner) patchManifest(ctx context.Context, owner string, manifestObj *v

func (r *Runner) updateAvailableManifestSpec(ctx context.Context, manifestObj *v1beta2.Manifest) error {
manifestInCluster := &v1beta2.Manifest{}
if err := r.Get(ctx, client.ObjectKey{
Namespace: manifestObj.GetNamespace(),
Name: manifestObj.GetName(),
}, manifestInCluster); err != nil {

if err := r.Get(ctx, client.ObjectKey{Namespace: manifestObj.GetNamespace(), Name: manifestObj.GetName()},
manifestInCluster); err != nil {
return fmt.Errorf("error get manifest %s: %w", client.ObjectKeyFromObject(manifestObj), err)
}
if !needToUpdate(manifestInCluster, manifestObj) {
return nil
}
manifestInCluster.Spec = manifestObj.Spec
if err := r.Update(ctx, manifestInCluster); err != nil {
return fmt.Errorf("error update manifest %s: %w", client.ObjectKeyFromObject(manifestObj), err)
}
return nil
}

func NeedToUpdate(manifestInCluster, manifestObj *v1beta2.Manifest, moduleStatus *v1beta2.ModuleStatus) bool {
if manifestInCluster == nil || moduleStatus == nil { // moduleStatus is nil in case of mandatory module
return true
}

return manifestObj.Spec.Version != moduleStatus.Version ||
manifestObj.Labels[shared.ChannelLabel] != moduleStatus.Channel ||
moduleStatus.State != manifestInCluster.Status.State
func needToUpdate(manifestInCluster, manifestObj *v1beta2.Manifest) bool {
return manifestInCluster.Spec.Version != manifestObj.Spec.Version
}

func (r *Runner) deleteManifest(ctx context.Context, module *common.Module) error {
Expand Down
82 changes: 0 additions & 82 deletions pkg/module/sync/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
apierrors "k8s.io/apimachinery/pkg/api/errors"
apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/pkg/module/sync"
"github.com/kyma-project/lifecycle-manager/pkg/testutils"
Expand Down Expand Up @@ -188,83 +186,3 @@ func configureModuleInKyma(
kyma.Status.Modules = append(kyma.Status.Modules, module)
}
}

func TestNeedToUpdate(t *testing.T) {
type args struct {
manifestInCluster *v1beta2.Manifest
manifestObj *v1beta2.Manifest
moduleStatus *v1beta2.ModuleStatus
}
tests := []struct {
name string
args args
want bool
}{
{
"When manifest in cluster is nil, expect need to update",
args{nil, &v1beta2.Manifest{}, &v1beta2.ModuleStatus{}},
true,
},
{
"When new module version available, expect need to update",
args{
&v1beta2.Manifest{},
&v1beta2.Manifest{
ObjectMeta: apimetav1.ObjectMeta{
Labels: map[string]string{shared.ChannelLabel: "regular"},
},
Spec: v1beta2.ManifestSpec{Version: "0.2"},
}, &v1beta2.ModuleStatus{Version: "0.1", Channel: "regular"},
},
true,
},
{
"When channel switch, expect need to update",
args{
&v1beta2.Manifest{},
&v1beta2.Manifest{
ObjectMeta: apimetav1.ObjectMeta{
Labels: map[string]string{shared.ChannelLabel: "fast"},
},
Spec: v1beta2.ManifestSpec{Version: "0.1"},
}, &v1beta2.ModuleStatus{Version: "0.1", Channel: "regular"},
},
true,
},
{
"When cluster Manifest in divergent state, expect need to update",
args{
&v1beta2.Manifest{Status: shared.Status{
State: "Warning",
}},
&v1beta2.Manifest{},
&v1beta2.ModuleStatus{State: "Ready"},
},
true,
},
{
"When no update required, expect no update",
args{
&v1beta2.Manifest{
Status: shared.Status{
State: "Ready",
},
Spec: v1beta2.ManifestSpec{Version: "0.1"},
},
&v1beta2.Manifest{
ObjectMeta: apimetav1.ObjectMeta{
Labels: map[string]string{shared.ChannelLabel: "regular"},
},
Spec: v1beta2.ManifestSpec{Version: "0.1"},
},
&v1beta2.ModuleStatus{State: "Ready", Version: "0.1", Channel: "regular"},
},
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, sync.NeedToUpdate(tt.args.manifestInCluster, tt.args.manifestObj, tt.args.moduleStatus), "needToUpdate(%v, %v, %v)", tt.args.manifestInCluster, tt.args.manifestObj, tt.args.moduleStatus)
})
}
}
5 changes: 0 additions & 5 deletions pkg/testutils/builder/moduletemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,6 @@ func ComponentDescriptorFactoryFromSchema(schemaVersion compdesc.SchemaVersion)
return moduleTemplate.Spec.Descriptor
}

func ReadComponentDescriptorFromFile(template string, moduleTemplate *v1beta2.ModuleTemplate) {
// needs to be encapsulated in an outside call to make the runtime.Caller(1) find the proper path
readComponentDescriptorFromYaml(template, moduleTemplate)
}

func readComponentDescriptorFromYaml(template string, moduleTemplate *v1beta2.ModuleTemplate) {
_, filename, _, ok := runtime.Caller(1)
if !ok {
Expand Down
50 changes: 31 additions & 19 deletions tests/integration/controller/kyma/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/genericocireg/componentmapping"
"github.com/open-component-model/ocm/pkg/runtime"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -26,7 +25,6 @@ import (
"github.com/kyma-project/lifecycle-manager/internal/pkg/flags"

. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
"github.com/kyma-project/lifecycle-manager/pkg/testutils/builder"
)

const (
Expand Down Expand Up @@ -93,26 +91,41 @@ var _ = Describe("Update Manifest CR", Ordered, func() {
WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient, shared.StateReady).
Should(Succeed())

By("Update Module Template spec")
var moduleTemplateFromFile v1beta2.ModuleTemplate
builder.ReadComponentDescriptorFromFile("operator_v1beta2_moduletemplate_kcp-module_updated.yaml", &moduleTemplateFromFile)
By("Update Module Template spec.data.spec field")
valueUpdated := "valueUpdated"
Eventually(updateKCPModuleTemplateSpecData(kyma.Name, valueUpdated), Timeout, Interval).Should(Succeed())

moduleTemplateInCluster := &v1beta2.ModuleTemplate{}
err := kcpClient.Get(ctx, client.ObjectKey{
Name: createModuleTemplateName(module),
Namespace: kyma.GetNamespace(),
}, moduleTemplateInCluster)
Expect(err).ToNot(HaveOccurred())
By("CR updated with new value in spec.resource.spec")
Eventually(expectManifestSpecDataEquals(kyma.Name, valueUpdated), Timeout, Interval).Should(Succeed())

moduleTemplateInCluster.Spec = moduleTemplateFromFile.Spec
By("Update Module Template spec.descriptor.component values")
{
newComponentDescriptorRepositoryURL := func(moduleTemplate *v1beta2.ModuleTemplate) error {
descriptor, err := descriptorProvider.GetDescriptor(moduleTemplate)
if err != nil {
return err
}

Eventually(kcpClient.Update, Timeout, Interval).
WithContext(ctx).
WithArguments(moduleTemplateInCluster).
Should(Succeed())
repositoryContext := descriptor.GetEffectiveRepositoryContext().Object
_, ok := repositoryContext["baseUrl"].(string)
if !ok {
Fail("Can't find \"baseUrl\" property in ModuleTemplate spec")
}
repositoryContext["baseUrl"] = updateRepositoryURL

By("CR updated with new value in spec.resource.spec")
Eventually(expectManifestSpecDataEquals(kyma.Name, "valueUpdated"), Timeout, Interval).Should(Succeed())
newDescriptorRaw, err := compdesc.Encode(descriptor.ComponentDescriptor, compdesc.DefaultJSONCodec)
Expect(err).ToNot(HaveOccurred())
moduleTemplate.Spec.Descriptor.Raw = newDescriptorRaw

return nil
}

updateKCPModuleTemplateWith := updateKCPModuleTemplate(module, kyma.Spec.Channel)
update := func() error {
return updateKCPModuleTemplateWith(newComponentDescriptorRepositoryURL)
}
Eventually(update, Timeout, Interval).Should(Succeed())
}

By("Manifest is updated with new value in spec.install.source")
{
Expand Down Expand Up @@ -196,7 +209,6 @@ var _ = Describe("Manifest.Spec is reset after manual update", Ordered, func() {

manifestImageSpec := extractInstallImageSpec(manifest.Spec.Install)
manifestImageSpec.Repo = updateRepositoryURL
manifest.Spec.Version = "v1.7.0" // required to allow for SSA of manifest

// is there a simpler way to update manifest.Spec.Install?
updatedBytes, err := json.Marshal(manifestImageSpec)
Expand Down

0 comments on commit 9d639f3

Please sign in to comment.