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

Add Support for specifying git branch and tag in devfile #4260

Merged
merged 7 commits into from
Dec 7, 2020

Conversation

adisky
Copy link
Contributor

@adisky adisky commented Nov 26, 2020

What type of PR is this?

/kind feature

What does does this PR do / why we need it:
Updates go-git library
Users can specify git branch and git tag using checkoutFrom in devfile, This PR implements the same

starterProjects:
  - name: odo
    git:
      remotes:
        origin: https://github.com/openshift/odo
      checkoutFrom:
        revision: v2.0.0
        remote: origin

Which issue(s) this PR fixes:

Fixes #4257

PR acceptance criteria:

How to test changes / Special notes to the reviewer:
Specify revision as shown above in devfile, revision could be a tag or branch. odo should download the starter project from specified branch/tag

@adisky adisky changed the title Add Support for specifying git branch and tag in devfile [WIP] Add Support for specifying git branch and tag in devfile Nov 26, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Nov 26, 2020
@adisky
Copy link
Contributor Author

adisky commented Nov 26, 2020

Need to write tests

Fix Integration test
@adisky
Copy link
Contributor Author

adisky commented Dec 1, 2020

/retest

Does not seemed to be linked error with the PR.

 When linking devfile component with Operator backed service
  /go/src/github.com/openshift/odo/tests/integration/operatorhub/cmd_service_test.go:360
    should successfully connect and disconnect a component with an existing service [It]
    /go/src/github.com/openshift/odo/tests/integration/operatorhub/cmd_service_test.go:410
    No future change is possible.  Bailing out early after 300.515s.
      
    Running odo with args [odo push]
    Expected
        <int>: 1
    to match exit code:
        <int>: 0
    /go/src/github.com/openshift/odo/tests/helper/helper_run.go:34
------------------------------

@dharmit
Copy link
Member

dharmit commented Dec 1, 2020

Does not seemed to be linked error with the PR.

 When linking devfile component with Operator backed service
  /go/src/github.com/openshift/odo/tests/integration/operatorhub/cmd_service_test.go:360
    should successfully connect and disconnect a component with an existing service [It]
    /go/src/github.com/openshift/odo/tests/integration/operatorhub/cmd_service_test.go:410
    No future change is possible.  Bailing out early after 300.515s.
      
    Running odo with args [odo push]
    Expected
        <int>: 1
    to match exit code:
        <int>: 0
    /go/src/github.com/openshift/odo/tests/helper/helper_run.go:34
------------------------------

The reason for failure shows before the summary of failure. Here are the relevant logs:

 [odo] I1201 07:54:17.997656   69701 deployments.go:139] Waiting for deployment "xpcvfl" rollout to finish: 0 of 1 updated replicas are available...
[odo] I1201 07:54:17.997662   69701 deployments.go:146] Waiting for deployment spec update to be observed...
[odo]  ✗  Waiting for component to start [5m]
[odo]  ✗  Failed to start component with name xpcvfl. Error: Failed to create the component: error while waiting for deployment rollout: timeout while waiting for xpcvfl deployment roll out
Deleting project: khbtduaeqe
Running odo with args [odo project delete khbtduaeqe -f]
[odo] I1201 07:59:17.399962   69728 util.go:736] HTTPGetRequest: https://raw.githubusercontent.com/openshift/odo/master/build/VERSION
[odo] I1201 07:59:17.400252   69728 util.go:757] Response will be cached in /tmp/odohttpcache for 1h0m0s
[odo] I1201 07:59:17.592408   69728 util.go:770] Cached response used.
[odo]  ⚠  Warning! Projects are deleted from the cluster asynchronously. Odo does its best to delete the project. Due to multi-tenant clusters, the project may still exist on a different node.
[odo] I1201 07:59:17.684440   69728 odo.go:72] Could not get the latest release information in time. Never mind, exiting gracefully :)
[odo]  ✓  Deleted project : khbtduaeqe
Setting current dir to: /go/src/github.com/openshift/odo/tests/integration/operatorhub
Deleting dir: /tmp/689072515
------------------------------
• Failure [368.286 seconds]
odo service command tests for OperatorHub
/go/src/github.com/openshift/odo/tests/integration/operatorhub/cmd_service_test.go:17
  When linking devfile component with Operator backed service
  /go/src/github.com/openshift/odo/tests/integration/operatorhub/cmd_service_test.go:360
    should successfully connect and disconnect a component with an existing service [It]
[Fail] odo service command tests for OperatorHub When linking devfile component with Operator backed service [It] should successfully connect and disconnect a component with an existing service 
/go/src/github.com/openshift/odo/tests/helper/helper_run.go:34
Ran 15 of 15 Specs in 823.713 seconds
FAIL! -- 14 Passed | 1 Failed | 0 Pending | 0 Skipped
--- FAIL: TestOperatorhub (823.72s)
FAIL

We're hitting #3256 like in many other recent CI failures. 😢

@adisky adisky changed the title [WIP] Add Support for specifying git branch and tag in devfile Add Support for specifying git branch and tag in devfile Dec 2, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Dec 2, 2020
@adisky
Copy link
Contributor Author

adisky commented Dec 2, 2020

/retest

level=error
level=error msg="Error: Error creating VPC: VpcLimitExceeded: The maximum number of VPCs has been reached."
level=error msg="\tstatus code: 400, request id: a8befd5a-4663-4987-864f-1f2b4d67c19c"
level=error
level=error msg="  on ../tmp/openshift-install-225736861/vpc/vpc.tf line 6, in resource \"aws_vpc\" \"new_vpc\":"
level=error msg="   6: resource \"aws_vpc\" \"new_vpc\" {"
level=error
level=error
level=fatal msg="failed to fetch Cluster: failed to generate asset \"Cluster\": failed to create cluster: failed to apply Terraform: failed to complete the change"
2020/12/02 07:31:55 Failed to upload $KUBECONFIG: timed out waiting for the condition: stat /tmp/secret/kubeconfig: no such file or directory
error: failed to execute wrapped command: exit status 1

@dharmit
Copy link
Member

dharmit commented Dec 2, 2020

Need to write tests

Is this ready for review?

@adisky
Copy link
Contributor Author

adisky commented Dec 3, 2020

Is this ready for review?

yes

Copy link
Member

@dharmit dharmit left a comment

Choose a reason for hiding this comment

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

Some trademark silly questions and some requests for adding more comments. Haven't tried checking the functionality yet.

pkg/odo/cli/component/create.go Show resolved Hide resolved
pkg/odo/cli/component/create.go Show resolved Hide resolved
refName = plumbing.NewBranchReferenceName(revision)
}

downloadSpinner := log.Spinnerf("Downloading starter project %s from %s", starterProject.Name, remoteUrl)
Copy link
Member

Choose a reason for hiding this comment

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

Using %q would print better output. I know we don't follow this across our code. So leaving it up to you to decide if you want to do it or not.

pkg/odo/cli/component/create.go Show resolved Hide resolved
pkg/odo/cli/component/create.go Show resolved Hide resolved
pkg/odo/cli/component/create.go Show resolved Hide resolved
@openshift-merge-robot
Copy link
Collaborator

openshift-merge-robot commented Dec 3, 2020

@adisky: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v4.5-integration-e2e 85c504b link /test v4.5-integration-e2e

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@dharmit
Copy link
Member

dharmit commented Dec 3, 2020

@adisky: The following tests failed, say /retest to rerun all failed tests:
Test name Commit Details Rerun command
ci/prow/v4.5-integration-e2e 85c504b link /test v4.5-integration-e2e
Full PR test history. Your PR dashboard.

/override ci/prow/v4.5-integration-e2e

@openshift-ci-robot
Copy link
Collaborator

@dharmit: /override requires a failed status context to operate on.
The following unknown contexts were given:

  • ci/prow/v4.5-integration-e2e

Only the following contexts were expected:

  • ci/prow/unit
  • ci/prow/v4.6-integration-e2e
  • tide

In response to this:

@adisky: The following tests failed, say /retest to rerun all failed tests:
Test name Commit Details Rerun command
ci/prow/v4.5-integration-e2e 85c504b link /test v4.5-integration-e2e
Full PR test history. Your PR dashboard.

/override ci/prow/v4.5-integration-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dharmit
Copy link
Member

dharmit commented Dec 7, 2020

@adisky I'm trying to check out the functionality and am facing a few issues. Maybe I'll ask a few embarrassing questions. First, I forked https://github.com/odo-devfiles/nodejs-ex repo and created a separate branch with minor changes. Here it is: dharmit/nodejs-ex@5d68d95.

Next, I built odo binary using your PR (well, obviously!)

Steps followed after that:

$ git clone repo
$ cd repo

$ odo create nodejs node

# next I modified the devfile and did `odo push`

On the URL, I see the message Hello from Node.js Starter Application! instead of the change I made. Here's the devfile:

devfile
commands:
- exec:
    commandLine: npm install
    component: runtime
    group:
      isDefault: true
      kind: build
    workingDir: /project
  id: install
- exec:
    commandLine: npm start
    component: runtime
    group:
      isDefault: true
      kind: run
    workingDir: /project
  id: run
- exec:
    commandLine: npm run debug
    component: runtime
    group:
      isDefault: true
      kind: debug
    workingDir: /project
  id: debug
- exec:
    commandLine: npm test
    component: runtime
    group:
      isDefault: true
      kind: test
    workingDir: /project
  id: test
components:
- container:
    endpoints:
    - name: http-3000
      targetPort: 3000
    - exposure: public
      name: node-3000
      path: /
      protocol: http
      targetPort: 3000
    image: registry.access.redhat.com/ubi8/nodejs-12:1-45
    memoryLimit: 1024Mi
    mountSources: true
    sourceMapping: /project
  name: runtime
metadata:
  name: nodejs
  version: 1.0.0
schemaVersion: 2.0.0
starterProjects:
- git:
    checkoutFrom:
      remote: origin
      revision: my-branch
    remotes:
      origin: https://github.com/dharmit/nodejs-ex.git
  name: nodejs-starter

What am I doing wrong?

@adisky
Copy link
Contributor Author

adisky commented Dec 7, 2020

devfile
What am I doing wrong?

This works when you download starter project with odo odo create nodejs --starter, you are cloning the project by yourself

Copy link
Member

@dharmit dharmit left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Dec 7, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adisky, dharmit

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 Dec 7, 2020
@openshift-merge-robot openshift-merge-robot merged commit c194899 into redhat-developer:master Dec 7, 2020
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. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odo requires the 'master' branch
4 participants