Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Replace Kyma CLI with modulectl in e2e #1966

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

lindnerby
Copy link
Member

Description

Changes proposed in this pull request:

  • Replace kyma cli action with modulectl
  • Replace cmd usage

Related issue(s)

@lindnerby lindnerby requested a review from a team as a code owner October 16, 2024 13:09
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 16, 2024
@lindnerby lindnerby requested a review from a team as a code owner October 16, 2024 15:08
@@ -19,7 +19,7 @@ runs:
shell: bash
run: |
make build-manifests
kyma alpha create module --module-config-file ./module-config.yaml --path . --registry localhost:5111 --insecure
module create --module-config-file ./module-config.yaml --registry http://localhost:5111 --insecure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to modify the module-config.yaml in the template-operator to have the defaultCR and manifest from the release URLs instead of the local file paths.

@@ -19,7 +19,7 @@ runs:
shell: bash
run: |
make build-manifests
kyma alpha create module --module-config-file ./module-config.yaml --path . --registry localhost:5111 --insecure
module create --module-config-file ./module-config.yaml --registry http://localhost:5111 --insecure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that we keep the Kyma CLI for this action since we agreed that the module teams will still be using the CLI until they have their modulereleasemeta in place. Also, this action will be removed after all modules are already using the modulereleasemeta.

Also, one more thing, using the modulectl generates the new format of the ModuleTemplate (and later on a different name ), and this will not be similar to the real modules' ModuleTemplates on landscapes

@@ -29,6 +32,7 @@ runs:
echo "k8s_version=${{ github.event.inputs.k8s_version || '1.30.3' }}" >> $GITHUB_OUTPUT
echo "istio_version=1.20.3" >> $GITHUB_OUTPUT
echo "k3d_version=5.7.4" >> $GITHUB_OUTPUT
echo "modulectl_version=1.0.0" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to document somewhere that we need to bump the modulectl version here when we do a new modulectl release, otherwise it might be forgotten?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if we want to use new features of it we need to bump it anyways?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but with the Kyma CLI we used the latest version, so it is a new thing to update the modulectl version for the E2E tests

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright. Any proposals where to document this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the modulectl release documented somewhere? Maybe we can add it there?

docs/contributor/04-local-test-setup.md Show resolved Hide resolved
@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. lgtm Looks good to me! size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants