-
Notifications
You must be signed in to change notification settings - Fork 54
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: Update devfile/api dependency to upstream commit a6c32fc #1184
Conversation
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
…er and kubernetes component Signed-off-by: Andrew Obuchowicz <[email protected]>
42d718c
to
1871cc1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love depending on a non-released version of devfile API, but it seems we have no choice until v2.2.1 is released.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amisevsk, AObuchow The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@amisevsk agreed. We could delay merging this if 2.2.1 is expected to be released soon enough. |
/retest |
Devfile v2.2.1 is now released: https://github.com/devfile/api/releases/tag/v2.2.1 Just took some asking questions :) |
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1184 +/- ##
==========================================
+ Coverage 52.66% 52.79% +0.13%
==========================================
Files 82 82
Lines 7497 7495 -2
==========================================
+ Hits 3948 3957 +9
+ Misses 3268 3259 -9
+ Partials 281 279 -2 ☔ View full report in Codecov by Sentry. |
@amisevsk awesome 🥳 I will rework this PR to update to the latest release |
Closing in favour of #1186 |
What does this PR do?
Updates the devfile/api dependency to the upstream commit devfile/api@a6c32fc. This is required for #1179, as the devfile validation library was previously preventing a deworkspace from having a container component and openshift/kubernetes component from using the same target port on an endpoint.
devfile/api@a6c32fc was chosen as it is the same commit used by the devfile library, and all commits past a6c32fc were non-function-changing commits: devfile/api@a6c32fc...e05a235
I also added a controller env test to test the case from #1179, but this might be redundant as this functionality should already be tested in the devfile API repo.
What issues does this PR fix or reference?
#1179
Is it tested? How?
Install DWO with the changes from this PR, and create a devworkspace with a container component and kubernetes (or openshift) component that uses the same endpoint target port, such as the following:
Ensure the workspace starts up as expected. You should see a pod get deployed to the
devworkspace-controller
namespace calledwebserver-dwo-deployed
. The main thing to ensure is the workspace does not fail to startup with the following reason:* devfile contains multiple containers with same endpoint targetPort: 8080
PR Checklist
/test v8-devworkspace-operator-e2e, v8-che-happy-path
to trigger)v8-devworkspace-operator-e2e
: DevWorkspace e2e testv8-che-happy-path
: Happy path for verification integration with CheSigned-off-by: Andrew Obuchowicz [email protected]