Fix the problem of attempts to build component w/o source #1341

anmolbabu · 2019-02-17T10:22:49Z

This PR starts supervisord as part of push which sets the stage
for push for example at the moment by creating the source dir
structure as configured also by the command config in
supervisor.conf

fixes #1066
Signed-off-by: anmolbabu anmolbudugutta@gmail.com

codeclimate · 2019-02-17T10:23:32Z

Code Climate has analyzed commit e352943 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1

View more on Code Climate.

anmolbabu · 2019-02-17T10:26:11Z

redhat-developer/odo-init-image#16 is a close relative of this PR

AppVeyorBot · 2019-02-17T10:32:11Z

✅ odo build 1629 completed and artifacts can be found here (for commit 2428317725 by @anmolbabu)

codecov · 2019-02-17T10:55:51Z

Codecov Report

Merging #1341 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1341      +/-   ##
==========================================
- Coverage   42.61%   42.55%   -0.06%     
==========================================
  Files          40       40              
  Lines        5050     5057       +7     
==========================================
  Hits         2152     2152              
- Misses       2683     2690       +7     
  Partials      215      215

Impacted Files	Coverage Δ
pkg/component/component.go	`19.27% <0%> (-0.32%)`	⬇️

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 d82c1ea...e352943. Read the comment docs.

amitkrout · 2019-02-18T08:28:10Z

@anmolbabu I am not sure, is #1341 (comment) a real threat ?

amitkrout · 2019-02-18T08:36:01Z

@anmolbabu Any thoughts you want to share on decrease in coverage. It looks straight forward to me in component.go file.

girishramnani

would be great to have a reliable regression test for this if possible.

kadel

If possible we should split s2i-setup and run from setup-and-run.
Setup could be run only when the container is started, and the Supervisor can just execute run script without setup.

cdrage · 2019-02-28T20:29:51Z

pkg/component/component.go

@@ -452,6 +452,15 @@ func PushLocal(client *occlient.Client, componentName string, applicationName st
 	}
 	targetPath := fmt.Sprintf("%s/src", s2iSrcPath)

+	stdin := bytes.NewBuffer([]byte{})
+	stdout := bytes.NewBuffer([]byte{})
+	stderr := bytes.NewBuffer([]byte{})


You can use this instead:

stdin, stdout, stderr := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{})

May look a bit nicer 👍

PR modified. Please revisit the PR @cdrage

cdrage · 2019-02-28T20:30:12Z

pkg/component/component.go

+	stdin := bytes.NewBuffer([]byte{})
+	stdout := bytes.NewBuffer([]byte{})
+	stderr := bytes.NewBuffer([]byte{})
+	err = client.ExecCMDInContainer(pod.Name, strings.Split("/var/lib/supervisord/bin/supervisord ctl start run", " "), stdout, stderr, stdin, false)


This seems super hacky, could we at least move the supervisord path to a constant?

PR modified but principle maintained Please revisit the PR @cdrage @surajnarwade

openshift-ci-robot · 2019-04-12T14:08:01Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: anmolbabu

If they are not already assigned, you can assign the PR to them by writing /assign @anmolbabu in a comment when ready.

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

anmolbabu · 2019-04-12T14:12:43Z

If possible we should split s2i-setup and run from setup-and-run.
Setup could be run only when the container is started, and the Supervisor can just execute run script without setup.

@kadel Done Please revisit the PR

anmolbabu · 2019-04-12T14:13:52Z

Tests are expected to fail as long as redhat-developer/odo-init-image#16 is not merged and version bumped

ToDo:

Bump the version of supervisord once Fix the problem of attempts to build component w/o source odo-init-image#16 is merged and released

surajnarwade · 2019-04-15T02:49:05Z

@anmolbabu , code lgtm, tests are failing on travis, I restarted those tests but I think you need to rebase once

This PR invokes setup script as part of push which sets the stage for push for example at the moment by creating the source dir structure as configured also by the command config in supervisor.conf fixes redhat-developer#1066 Signed-off-by: anmolbabu <anmolbudugutta@gmail.com>

openshift-ci-robot · 2019-04-17T10:24:07Z

@anmolbabu: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-core	`fb77124`	link	`/test e2e-core`
ci/prow/e2e-scenarios	`fb77124`	link	`/test e2e-scenarios`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

amitkrout · 2019-05-07T07:03:23Z

ping...@girishramnani who is going to address this pr ?

girishramnani · 2019-05-10T10:02:06Z

I will take a look at this

openshift-ci-robot · 2019-05-12T02:48:13Z

@anmolbabu: PR needs rebase.

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.

kadel · 2019-06-11T11:58:56Z

I will take a look at this

@girishramnani any progress on this?

amitkrout · 2019-08-20T11:17:03Z

ping @girishramnani

kadel · 2019-08-26T14:10:18Z

@girishramnani do we have any progress on this?

girishramnani · 2019-08-27T07:17:20Z

not yet, busy on the port forward stuff

girishramnani · 2019-12-11T20:26:23Z

a separate PR was opened and hence closing this PR.

anmolbabu requested a review from mohammedzee1000 as a code owner February 17, 2019 10:22

anmolbabu requested review from cdrage, kadel, metacosm, surajnarwade, mik-dass and syamgk February 17, 2019 10:23

anmolbabu requested a review from geoand February 17, 2019 10:23

anmolbabu mentioned this pull request Feb 17, 2019

Fix the problem of attempts to build component w/o source redhat-developer/odo-init-image#16

Closed

girishramnani reviewed Feb 19, 2019

View reviewed changes

kadel requested changes Feb 20, 2019

View reviewed changes

cdrage suggested changes Feb 28, 2019

View reviewed changes

kadel 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 Mar 14, 2019

anmolbabu force-pushed the fixes#1066 branch from e352943 to 70aa63b Compare April 12, 2019 12:59

openshift-ci-robot added the size/M label Apr 12, 2019

anmolbabu force-pushed the fixes#1066 branch 3 times, most recently from c963000 to d1c3033 Compare April 12, 2019 14:07

anmolbabu 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 Apr 12, 2019

surajnarwade added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 15, 2019

anmolbabu force-pushed the fixes#1066 branch from d1c3033 to 2720bd1 Compare April 15, 2019 13:54

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 15, 2019

anmolbabu force-pushed the fixes#1066 branch from 2720bd1 to fb77124 Compare April 15, 2019 13:55

girishramnani self-assigned this May 10, 2019

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label May 12, 2019

amitkrout removed request for metacosm, geoand, syamgk and surajnarwade August 20, 2019 11:17

girishramnani closed this Dec 11, 2019

rm3l added the estimated-size/M (10-20) Rough sizing for Epics. About 1 sprint of work for one person label Jun 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the problem of attempts to build component w/o source #1341

Fix the problem of attempts to build component w/o source #1341

anmolbabu commented Feb 17, 2019

codeclimate bot commented Feb 17, 2019

anmolbabu commented Feb 17, 2019

AppVeyorBot commented Feb 17, 2019

codecov bot commented Feb 17, 2019 •

edited

Loading

amitkrout commented Feb 18, 2019 •

edited

Loading

amitkrout commented Feb 18, 2019

girishramnani left a comment

kadel left a comment

cdrage Feb 28, 2019

anmolbabu Apr 12, 2019

cdrage Feb 28, 2019

surajnarwade Apr 10, 2019

anmolbabu Apr 12, 2019

openshift-ci-robot commented Apr 12, 2019

anmolbabu commented Apr 12, 2019

anmolbabu commented Apr 12, 2019

surajnarwade commented Apr 15, 2019

openshift-ci-robot commented Apr 17, 2019

amitkrout commented May 7, 2019

girishramnani commented May 10, 2019

openshift-ci-robot commented May 12, 2019

kadel commented Jun 11, 2019

amitkrout commented Aug 20, 2019

kadel commented Aug 26, 2019

girishramnani commented Aug 27, 2019

girishramnani commented Dec 11, 2019

Fix the problem of attempts to build component w/o source #1341

Fix the problem of attempts to build component w/o source #1341

Conversation

anmolbabu commented Feb 17, 2019

codeclimate bot commented Feb 17, 2019

anmolbabu commented Feb 17, 2019

AppVeyorBot commented Feb 17, 2019

codecov bot commented Feb 17, 2019 • edited Loading

Codecov Report

amitkrout commented Feb 18, 2019 • edited Loading

amitkrout commented Feb 18, 2019

girishramnani left a comment

Choose a reason for hiding this comment

kadel left a comment

Choose a reason for hiding this comment

cdrage Feb 28, 2019

Choose a reason for hiding this comment

anmolbabu Apr 12, 2019

Choose a reason for hiding this comment

cdrage Feb 28, 2019

Choose a reason for hiding this comment

surajnarwade Apr 10, 2019

Choose a reason for hiding this comment

anmolbabu Apr 12, 2019

Choose a reason for hiding this comment

openshift-ci-robot commented Apr 12, 2019

anmolbabu commented Apr 12, 2019

anmolbabu commented Apr 12, 2019

surajnarwade commented Apr 15, 2019

openshift-ci-robot commented Apr 17, 2019

amitkrout commented May 7, 2019

girishramnani commented May 10, 2019

openshift-ci-robot commented May 12, 2019

kadel commented Jun 11, 2019

amitkrout commented Aug 20, 2019

kadel commented Aug 26, 2019

girishramnani commented Aug 27, 2019

girishramnani commented Dec 11, 2019

codecov bot commented Feb 17, 2019 •

edited

Loading

amitkrout commented Feb 18, 2019 •

edited

Loading