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

3923- url creation with optional port flag #3950

Conversation

yangcao77
Copy link
Contributor

What type of PR is this?

/kind bug
/area devfile
/area url

What does does this PR do / why we need it:
The --port flag should be an optional flag if there is only one port exposed in devfile.
This PR fixes the regression.

Also created an integration test for the specific case

Which issue(s) this PR fixes:

Fixes #3923

PR acceptance criteria:

How to test changes / Special notes to the reviewer:
make test-cmd-devfile-url

Manual Test:
odo url create should successfully create an endpoint in devfile

Signed-off-by: Stephanie <yangcao@redhat.com>
Signed-off-by: Stephanie <yangcao@redhat.com>
@openshift-ci-robot openshift-ci-robot added kind/bug Categorizes issue or PR as related to a bug. area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. area/url labels Sep 14, 2020
Signed-off-by: Stephanie <yangcao@redhat.com>
@yangcao77 yangcao77 changed the title 3923 ur lcreation withoptional port 3923- url creation with optional port flag Sep 14, 2020
helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), context)
helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile.yaml"), filepath.Join(context, "devfile.yaml"))

helper.CmdShouldPass("odo", "url", "create", url1, "--host", host, "--secure", "--ingress")
Copy link
Contributor

@mik-dass mik-dass Sep 15, 2020

Choose a reason for hiding this comment

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

please add a check to verify that the default port was added to the devfile. We can check the devfile.yaml or use odo url list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@codecov
Copy link

codecov bot commented Sep 15, 2020

Codecov Report

Merging #3950 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3950   +/-   ##
=======================================
  Coverage   44.60%   44.60%           
=======================================
  Files         143      143           
  Lines       14000    14000           
=======================================
  Hits         6244     6244           
  Misses       7164     7164           
  Partials      592      592           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b055c99...4d86553. Read the comment docs.

Signed-off-by: Stephanie <yangcao@redhat.com>

helper.CmdShouldPass("odo", "url", "create", url1, "--host", host, "--ingress")
stdout := helper.CmdShouldPass("odo", "url", "list")
helper.MatchAllInOutput(stdout, []string{url1, "Not Pushed"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also check the port number in the output

Signed-off-by: Stephanie <yangcao@redhat.com>
@yangcao77
Copy link
Contributor Author

/retest

Copy link
Contributor

@mik-dass mik-dass left a comment

Choose a reason for hiding this comment

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

/lgtm

@mik-dass
Copy link
Contributor

error: some steps failed:
  * could not run steps: execution cancelled
time="2020-09-16T17:52:27Z" level=info msg="Reporting job state 'failed' with reason 'executing_graph:interrupted'"
{"component":"entrypoint","file":"prow/entrypoint/run.go:247","func":"k8s.io/test-infra/prow/entrypoint.gracefullyTerminate","level":"error","msg":"Process gracefully exited before 30m0s grace period","severity":"error","time":"2020-09-16T17:52:27Z"}

/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 17, 2020
@girishramnani
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Sep 17, 2020
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

6 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

12 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 3e7c89c into redhat-developer:master Sep 19, 2020
@rm3l rm3l added the v2 Issue or PR that applies to the v2 of odo label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. Required by Prow. v2 Issue or PR that applies to the v2 of odo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odo url create shouldn't require a port if only one port exists in the devfile
7 participants