KOGITO-7579: Knative Workflow - Specify Kogito version #1114

ljmotta · 2022-07-21T12:10:26Z

JIRAS

KOGITO-7579: Specify Kogito version
KOGITO-7485: macOS ARM build
KOGITO-7573: Verify image name
KOGITO-7607: Use Maven instead of Maven wrapper

On this PR

This PR adds the possibility to tweak the Kogito and Quarkus versions. Also, it adds some general improvements:

Removed the use of the mvnw and mvnw.cmd.
Check image name during build.
Build improvements:
- Added macOS ARM cross-compilation.
- Simplified staging and release build.

Specify Kogito and Quarkus version

During the project creation, it's possible to pass a --quarkus-version or the --kogito-version flags in the create command to set a specific Quarkus version and a Kogito version for the project.

The create command now generates a workflow.config.yml file containing the versions, and those versions can be tweaked at any time.

New commands

version
Show the plugin version:

kn workflow version

config
Tweak the config file and apply those changes in the pom.xml:

kn workflow config --quarkus-version=2.10.0.Final --kogito-version=1.24.0.Final

Apply manual changes in the pom.xml:

kn workflow config --apply

ljmotta · 2022-07-22T12:02:32Z

Hi @spolti ! I've added you as a reviewer as I think you can add insightful feedback.

@ricardozanini If you could take a look too, it would be great.

ricardozanini

Great work!

I might be missing something, but I'm missing the repository configuration. If the user sets the version like quarkus-redhat-000x, how will maven download it?

spolti

very nice, just a few things.

packages/kn-plugin-workflow/pkg/command/build.go

spolti · 2022-07-22T15:24:26Z

packages/kn-plugin-workflow/pkg/common/helper.go

+}
+
+func RunExtensionCommand(verbose bool, extensionCommand string, friendlyMessages []string, extensions string) error {
+	command := exec.Command("mvn", extensionCommand, fmt.Sprintf("-Dextensions=%s", extensions))


how to check if the mvn is available in the path?

Good catch. I should have checked the mvn and Java before the command starts with the CheckJavaDependencies function that is in the check.go file. I've forgotten to add this function call in the config command.

spolti · 2022-07-22T15:26:47Z

packages/kn-plugin-workflow/pkg/command/build.go

+func checkImageName(name string) (err error) {
+	matched, err := regexp.MatchString("[a-z]([-a-z0-9]*[a-z0-9])?", name)
+	if !matched {
+		fmt.Println("ERROR: Image name should match [a-z]([-a-z0-9]*[a-z0-9])?")


Do you mind to add a example about what this regex should match? Users might find it difficult to read regex.

spolti · 2022-07-22T15:27:35Z

packages/kn-plugin-workflow/pkg/command/config.go

+	"github.com/spf13/cobra"
+)
+
+type ConfigCmdConfig struct {


maybe rename to CmdConfig only?

I've adopt the "CmdConfig" name pattern. So the other commands names are "BuildCmdConfig", "CreateCmdConfig", etc. ConfigCmdConfig it's not pretty, but it's what it is for this case.

spolti · 2022-07-22T15:29:25Z

packages/kn-plugin-workflow/pkg/command/config.go

+	return nil
+}
+
+func runConfigCmdConfig(cmd *cobra.Command) (cfg ConfigCmdConfig, err error) {


maybe rename?

spolti · 2022-07-22T15:29:47Z

packages/kn-plugin-workflow/pkg/command/config.go

+	return
+}
+
+func getUpdateExtensionFriendlyMessages() []string {


isn't this duplicated?

No. I'm going to centralize all messages, I think it's going to be better to maintain it.

packages/kn-plugin-workflow/pkg/command/config_workflow.go

ederign · 2022-07-22T19:05:51Z

" might be missing something, but I'm missing the repository configuration. If the user sets the version like quarkus-redhat-000x, how will maven download it?"

@ricardozanini , for now, to overcomplicate it, the user will have to have global settings configured. (This is probably the case if they use an RH distribution in their company).

ljmotta · 2022-07-25T19:49:33Z

@spolti Hi, I believe I've covered all raised topics, do you have any other additional comment?

@barboras7 Could you please take a look into this PR?

barboras7 · 2022-07-26T06:50:52Z

Hi @ljmotta,
sure. Sorry for the delay, Mondays are mostly meetings for me. I'm on it now!

barboras7

Hi @ljmotta,
great work! I am leaving some minor comments below.

Also, one thing I noticed when testing the plugin manually:
When I create a project using kn workflow create --name myTesting, I am able to do so. However, when I want to build this, I need to use mytesting instead of myTesting to succeed. Do you think you could add a validation for project creation to avoid such issues?

barboras7 · 2022-07-26T17:48:39Z

packages/kn-plugin-workflow/cmd/main.go

+			QuarkusVersion: quarkusVersion,
+			KogitoVersion:  kogitoVersion,
+		},
+		Version: version,


It might be difficult in the future to understand what version is. Would it be possible to use a more specific variable name?

barboras7 · 2022-07-26T17:52:43Z

packages/kn-plugin-workflow/pkg/command/build.go

 	}

-	fmt.Println("✅ Quarkus extension was successfully add to the project")
+	fmt.Println("✅ Quarkus extension was successfully add to the project pom.xml")


Suggested change

fmt.Println("✅ Quarkus extension was successfully add to the project pom.xml")

fmt.Println("✅ Quarkus extension was successfully added to the project pom.xml")

barboras7 · 2022-07-26T18:04:15Z

packages/kn-plugin-workflow/pkg/command/config_workflow.go

+		return
+	}
+
+	// check if quarkus and kogito versions in the config file


Suggested change

// check if quarkus and kogito versions in the config file

// check if quarkus and kogito versions are in the config file

ljmotta · 2022-07-26T20:04:11Z

@barboras7 Sure thing. I'm going to add the validation and make the suggested changes. Thanks for reviewing.

ljmotta · 2022-07-26T20:39:14Z

@barboras7 Actually, after I've started making the changes, I've realized that adding this validation in the create command wouldn't be the ideal. The create command creates a local folder for the user and each user have their own organization. The build generates a container image, therefore has its own name limitations and patterns, and don't need to follow the folder/project name.

barboras7 · 2022-07-27T10:51:40Z

Thanks @ljmotta, I understand why you don't feel like the validation would be ideal. Maybe just a small suggestion - a log message at a warning level, when project name does not follow a correct image name pattern like To build an image, you need to use an image name matching the following regex .... or something similar. Anyway, that's just a suggestion. Great work! 🥳

* Parametrize Kogito version * Remove versions from staging and release build, and minor fixes * Add version and config file * Use env to set a custom quarkus and kogito version * Check env in main * Fix env * Add config file * Create version env * Extract file names to const * Cross compile to arm (macOS M1) * Add arm build script * Fix readme * Update the build extension version * Minor tweaks * Clean up * Remove both build extensions * Update dependencies versions in the config command * Add apply option * Update project version * Bumb up golang version * Update go.mod * PR review * Explain runAddExtension * Review

ljmotta added 20 commits July 15, 2022 17:29

Parametrize Kogito version

45322f8

Remove versions from staging and release build, and minor fixes

b874318

Add version and config file

c197016

Use env to set a custom quarkus and kogito version

f99985e

Check env in main

07ee85f

Fix env

ade509a

Add config file

ee30b7b

Create version env

2c31d20

Merge remote-tracking branch 'origin/main' into kogito-7579

c2b30c7

Extract file names to const

e18b355

Cross compile to arm (macOS M1)

7e0a0cc

Add arm build script

6ce227b

Fix readme

cc50f09

Update the build extension version

91614e0

Minor tweaks

a556a23

Clean up

b494b92

Remove both build extensions

6db04ca

Update dependencies versions in the config command

9f12009

Add apply option

ef63c5e

Update project version

52e014a

ljmotta requested review from ederign and tiagobento as code owners July 21, 2022 12:10

tiagobento added the area:sonataflow label Jul 21, 2022

Bumb up golang version

e2174bc

ljmotta force-pushed the kogito-7579 branch from bae3634 to e2174bc Compare July 21, 2022 13:22

tiagobento added the pr: order: 1 label Jul 21, 2022

Update go.mod

354568f

ljmotta requested review from spolti and barboras7 July 22, 2022 12:02

ricardozanini reviewed Jul 22, 2022

View reviewed changes

spolti reviewed Jul 22, 2022

View reviewed changes

ljmotta added 2 commits July 22, 2022 17:27

PR review

85e3411

Explain runAddExtension

b352e74

tiagobento removed the pr: order: 1 label Jul 25, 2022

tiagobento mentioned this pull request Jul 25, 2022

Make build-env configurable per package #1116

Merged

Merge remote-tracking branch 'origin/main' into kogito-7579

52eaf25

ljmotta force-pushed the kogito-7579 branch from 759d047 to 52eaf25 Compare July 26, 2022 13:33

barboras7 reviewed Jul 26, 2022

View reviewed changes

Review

0a0274e

barboras7 approved these changes Jul 27, 2022

View reviewed changes

spolti approved these changes Jul 27, 2022

View reviewed changes

tiagobento approved these changes Jul 27, 2022

View reviewed changes

tiagobento merged commit db857a3 into apache:main Jul 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KOGITO-7579: Knative Workflow - Specify Kogito version #1114

KOGITO-7579: Knative Workflow - Specify Kogito version #1114

ljmotta commented Jul 21, 2022

ljmotta commented Jul 22, 2022

ricardozanini left a comment

spolti left a comment

spolti Jul 22, 2022

ljmotta Jul 22, 2022 •

edited

Loading

spolti Jul 22, 2022

ljmotta Jul 22, 2022

spolti Jul 22, 2022

ljmotta Jul 22, 2022

spolti Jul 22, 2022

spolti Jul 22, 2022

ljmotta Jul 22, 2022

ederign commented Jul 22, 2022

ljmotta commented Jul 25, 2022

barboras7 commented Jul 26, 2022

barboras7 left a comment

barboras7 Jul 26, 2022

barboras7 Jul 26, 2022

barboras7 Jul 26, 2022

ljmotta commented Jul 26, 2022

ljmotta commented Jul 26, 2022

barboras7 commented Jul 27, 2022

	fmt.Println("✅ Quarkus extension was successfully add to the project pom.xml")
	fmt.Println("✅ Quarkus extension was successfully added to the project pom.xml")

	// check if quarkus and kogito versions in the config file
	// check if quarkus and kogito versions are in the config file

KOGITO-7579: Knative Workflow - Specify Kogito version #1114

KOGITO-7579: Knative Workflow - Specify Kogito version #1114

Conversation

ljmotta commented Jul 21, 2022

JIRAS

On this PR

Specify Kogito and Quarkus version

New commands

ljmotta commented Jul 22, 2022

ricardozanini left a comment

Choose a reason for hiding this comment

spolti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljmotta Jul 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ederign commented Jul 22, 2022

ljmotta commented Jul 25, 2022

barboras7 commented Jul 26, 2022

barboras7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljmotta commented Jul 26, 2022

ljmotta commented Jul 26, 2022

barboras7 commented Jul 27, 2022

ljmotta Jul 22, 2022 •

edited

Loading