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

feat(ci): latest runtime snapshot available #3764

Merged
merged 7 commits into from
Oct 26, 2022

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Oct 21, 2022

With this PR we aim to test Camel K main branch against the latest produced in snapshot by Camel K runtime main. In this way we don't have to wait the end of a release cycle to have Camel K Runtime development feedback and we may discover issues immediately when adding any change to the runtime. If we're happy with this approach, we may extend it to release branches as well and ensure that a snapshot is always pushed by Camel K runtime project.

Release Note

feat(ci): latest runtime snapshot available

@squakez
Copy link
Contributor Author

squakez commented Oct 21, 2022

@claudio4j @oscerd @tadayosi wdyt?

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

It's a good idea, just one minor finding. We also need to be sure the deploy on snapshot always work. We need to set up a github action or ci build, to always deploy camel-k-runtime to snapshot. Every night for example.

script/maven-settings.xml Outdated Show resolved Hide resolved
@squakez
Copy link
Contributor Author

squakez commented Oct 21, 2022

It's a good idea, just one minor finding. We also need to be sure the deploy on snapshot always work. We need to set up a github action or ci build, to always deploy camel-k-runtime to snapshot. Every night for example.

Yes, that's correct. We already push a snapshot for Camel K runtime when we add a commit, but, it would be advisable to enable the same nightly to refresh that.

@oscerd
Copy link
Contributor

oscerd commented Oct 21, 2022

All good then. At some point we'll need to move the runtime bits in camel quarkus, so the camel k release will be easy

@squakez
Copy link
Contributor Author

squakez commented Oct 21, 2022

Hey @oscerd, right now the publish of Camel K runtime on snapshot always happen when a new commit is pushed to the main or release branches. If we make sure to double check any error when we merge, then, we should have a guarantee that the latest changes are in the snapshot. I'd say that it would be okey for the time being. Nevertheless, I'll create a follow up request on that project to setup a nightly snapshot, but I think we can work with a lower priority.

@claudio4j
Copy link
Contributor

As there is this maven-settings.xml, the other scripts maven_overlay.sh, package_maven_artifacts.sh, can be changed to use this maven-settings.xml.

currently the make IMAGE_NAME=apache/camel-k images-dev fails with

mkdir -p build/_maven_overlay
./script/maven_overlay.sh -s "" -d "" 1.16.0-SNAPSHOT build/_maven_overlay
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:2.8:copy (default-cli) on project standalone-pom: Unable to find artifact.: org.apache.camel.k:camel-k-maven-logging:zip:1.16.0-SNAPSHOT was not found in https://maven.repository.redhat.com/earlyaccess/all/ during a previous attempt.
`` 

@squakez
Copy link
Contributor Author

squakez commented Oct 21, 2022

Yes, we need this as well: apache/camel-k-runtime#911

@tadayosi
Copy link
Member

As I remember it we had been using the runtime snapshots in earlier Camel K versions. I don't know why we haven't done the same lately. Definitely a good idea.

@squakez
Copy link
Contributor Author

squakez commented Oct 25, 2022

Some error in the local test:

❌ TestLocalBuild (5m0.05s)
❌ TestLocalBuildDependenciesOnly (1m0.06s)
❌ TestLocalBuildIntegrationDirectory (1m0.06s)
❌ TestLocalBuildIntegrationDirectoryWithMultiBytes (1m0.06s)
❌ TestLocalBuildIntegrationDirectoryWithSpaces (1m0.06s)
❌ TestLocalBuildModelineDependencies (1m0.06s)
✅ TestLocalBuildWithInvalidDependency (1.5s)
❌ TestLocalBuildWithTrait (5m0.07s)
✅ TestLocalInspect (140ms)
✅ TestLocalInspectWithDependencies (140ms)
❌ TestLocalRun (5m0.07s)
❌ TestLocalRunContainerize (0s)
❌ TestLocalRunWithDependencies (5m0.06s)
✅ TestLocalRunWithInvalidDependency (1.55s)

@tadayosi does it ring any bell?

@tadayosi
Copy link
Member

@squakez Looking at the error output, they are failing because they cannot execute a local Maven project to calculate dependencies. It appears there are some problems in downloading and resolving dependencies with a snapshot runtime.

Error: failed to compute transitive dependencies: failure while building project: exit status 1

You should be able to replicate the issue locally by kamel local run with a snapshot runtime.

@tadayosi
Copy link
Member

I can replicate it locally:

$ ./kamel local build e2e/local/files/yaml.yaml --integration-directory test
Error: failed to compute transitive dependencies: failure while building project: exit status 1

@tadayosi
Copy link
Member

I think the root cause is the the default Maven repositories don't include https://repository.apache.org/content/repositories/snapshots-group. We'd need to find a way to include the snapshot one to the default repos only when we are in development and use a snapshot repo.

@tadayosi
Copy link
Member

tadayosi commented Oct 26, 2022

One more issue to fix is that the current maven util impl doesn't consider snapshot repos, so even if you use --maven-repository flag it won't work.
https://github.com/apache/camel-k/blob/main/pkg/util/maven/maven_repositories.go#L31-L76

$ ./kamel local run e2e/local/files/yaml.yaml --maven-repository=https://repository.apache.org/content/repositories/snapshots-group
Error: failed to compute transitive dependencies: failure while building project: exit status 1

EDIT:
Ah I didn't know --maven-repository=https://repository.apache.org/content/repositories/snapshots-group@snapshots works!

@squakez
Copy link
Contributor Author

squakez commented Oct 26, 2022

Thank you so much @tadayosi. I'll have a look and fix accordingly.

@squakez
Copy link
Contributor Author

squakez commented Oct 26, 2022

I've fixed the local problems and rebased with the new timeout increased. 🤞

Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

Fixing e2e/local/util.go should pass the local-it tests but a compiled kamel from source won't still work with ./kamel local run. I think we need to fix the main pkg source as well.

e2e/local/util.go Outdated Show resolved Hide resolved
@squakez squakez closed this Oct 26, 2022
@squakez squakez reopened this Oct 26, 2022
@squakez
Copy link
Contributor Author

squakez commented Oct 26, 2022

Fixing e2e/local/util.go should pass the local-it tests but a compiled kamel from source won't still work with ./kamel local run. I think we need to fix the main pkg source as well.

Why not? I mean, you need to provide --maven-repository https://repository.apache.org/content/repositories/snapshots-group@snapshots or to expect those snapshot to be already available in your local machine. But, a part that detail, the execution should work as expected.

@tadayosi
Copy link
Member

tadayosi commented Oct 26, 2022

Why not? I mean, you need to provide --maven-repository https://repository.apache.org/content/repositories/snapshots-group@snapshots or to expect those snapshot to be already available in your local machine. But, a part that detail, the execution should work as expected.

Yes, so from now on, we need to remeber to add --maven-repository https://repository.apache.org/content/repositories/snapshots-group@snapshots whenever you want to test kamel local with a locally built kamel cli. I was thinking if there's a smart way to add the snapshot repo to the default repos set so that we won't need to add --maven-repository manually during development.

But we can improve this in a separate pull req.

@squakez
Copy link
Contributor Author

squakez commented Oct 26, 2022

It looks good now, the failing checks appears to be something flaky.

@squakez squakez merged commit a16aaa2 into apache:main Oct 26, 2022
@squakez squakez deleted the feat/ci_runtime_snapshot branch October 26, 2022 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants