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

PR got merged even when TravisCI test were failing #2019

Closed
kadel opened this issue Aug 14, 2019 · 31 comments · Fixed by openshift/release#11166
Closed

PR got merged even when TravisCI test were failing #2019

kadel opened this issue Aug 14, 2019 · 31 comments · Fixed by openshift/release#11166
Assignees
Labels
area/refactoring Issues or PRs related to code refactoring area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering estimated-size/L (20-40) Rough sizing for Epics. About 2 sprints of work for a person. priority/Medium Nice to have issue. Getting it done before priority changes would be great.

Comments

@kadel
Copy link
Member

kadel commented Aug 14, 2019

on travis CI
https://travis-ci.com/openshift/odo/jobs/225078001

notice the projects names

Running odo with args [odo service create dh-postgresql-apb --plan dev -p postgresql_user=luke -p postgresql_password=secret -p postgresql_database=my_data -p postgresql_version=9.6 --app uphmxpo --project ajdvqxarqf]

[odo]  ✗  Component dh-postgresql-apb does not exist in application uphmxpo

full output

Creating a new project: ajdvqxarqf
Running odo with args [odo project create ajdvqxarqf -w -v4]
[odo] I0814 08:14:02.342870   23023 preference.go:116] The configFile is /home/travis/.odo/preference.yaml
[odo] I0814 08:14:02.343241   23023 occlient.go:478] Trying to connect to server 127.0.0.1:8443
[odo] I0814 08:14:02.346659   23023 occlient.go:485] Server https://127.0.0.1:8443 is up
[odo] I0814 08:14:02.461155   23023 occlient.go:408] isLoggedIn err:  <nil> 
[odo]  output: "developer"
[odo] I0814 08:14:02.461193   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: context
[odo] I0814 08:14:02.461203   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: context
[odo] I0814 08:14:02.461236   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: app
[odo] I0814 08:14:02.461243   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: project
[odo] I0814 08:14:02.471270   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: app
[odo] I0814 08:14:02.471297   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: output
[odo] I0814 08:14:02.471302   23023 context.go:371] Ignoring error as it usually means flag wasn't set: flag accessed but not defined: component
[odo]  •  Waiting for project to come up  ...
[odo]  ✓  Waiting for project to come up [1s]
[odo]  ✓  Project 'ajdvqxarqf' is ready for use
[odo]  ✓  New project created and now using project : ajdvqxarqf
[odo] I0814 08:14:03.650283   23023 odo.go:70] Could not get the latest release information in time. Never mind, exiting gracefully :)
Created dir: /tmp/659686430
Current working dir: /home/travis/gopath/src/github.com/openshift/odo/tests/integration
Setting current dir to: /tmp/659686430
Setting current dir to: /home/travis/gopath/src/github.com/openshift/odo/tests/integration
Running odo with args [odo service create dh-postgresql-apb --plan dev -p postgresql_user=luke -p postgresql_password=secret -p postgresql_database=my_data -p postgresql_version=9.6 --app uphmxpo --project ajdvqxarqf]
[odo]  •  Creating service  ...
[odo]  ✓  Creating service [183ms]
[odo]  ✓  Service 'dh-postgresql-apb' was created
[odo] Progress of the provisioning will not be reported and might take a long time.
[odo] You can see the current status by executing 'odo service list'
Running oc with args [oc get serviceinstance -o name -n ajdvqxarqf]
[oc] serviceinstance.servicecatalog.k8s.io/dh-postgresql-apb
Running odo with args [odo service list --app uphmxpo --project ajdvqxarqf]
[odo] NAME                  TYPE                  STATUS
[odo] dh-postgresql-apb     dh-postgresql-apb     Provisioning
Running odo with args [odo delete -f dh-postgresql-apb --app uphmxpo --project ajdvqxarqf]
[odo]  ✗  Component dh-postgresql-apb does not exist in application uphmxpo
Deleting project: ajdvqxarqf
Running odo with args [odo project delete ajdvqxarqf -f]
[odo] This project contains the following applications, which will be deleted
[odo] Application uphmxpo
[odo] This application has following service that will be deleted
[odo] service named dh-postgresql-apb of type dh-postgresql-apb
[odo]  •  Deleting project ajdvqxarqf  ...
[odo]  ✓  Deleting project ajdvqxarqf [8s]
[odo] Deleted project : ajdvqxarqf
Deleting dir: /tmp/659686430
Setting current dir to: /home/travis/gopath/src/github.com/openshift/odo/tests/integration
• Failure [15.788 seconds]
odoServiceE2e
/home/travis/gopath/src/github.com/openshift/odo/tests/integration/service_test.go:15
  When working from outside a component dir
  /home/travis/gopath/src/github.com/openshift/odo/tests/integration/service_test.go:105
    should be able to create, list and delete services without a context and using --app and --project flags instaed [It]
    /home/travis/gopath/src/github.com/openshift/odo/tests/integration/service_test.go:143
    No future change is possible.  Bailing out early after 0.723s.
    Running odo with args [odo delete -f dh-postgresql-apb --app uphmxpo --project ajdvqxarqf]
    Expected
        <int>: 1
    to match exit code:
        <int>: 0
    /home/travis/gopath/src/github.com/openshift/odo/tests/helper/helper_run.go:32
@kadel kadel added the flake Categorizes issue or PR as related to a flaky test. label Aug 14, 2019
@amitkrout
Copy link
Contributor

@kadel I could not find any abnormality in project name ajdvqxarqf. But the part that stuck to mind is service list STATUS

Running odo with args [odo service list --app uphmxpo --project ajdvqxarqf]
[odo] NAME                  TYPE                  STATUS
[odo] dh-postgresql-apb     dh-postgresql-apb     Provisioning

It seems the service is not created properly because the SATATUS is still showing Provisioning. Then immediately in the very next step deletion of service is invoked and causes the failure

Running odo with args [odo delete -f dh-postgresql-apb --app uphmxpo --project ajdvqxarqf]
[odo]  ✗  Component dh-postgresql-apb does not exist in application uphmxpo

Test is hitting the issue because helper.WaitForCmdOut is looking for the service name instead of service status.

Will send a pr

@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

It seems the service is not created properly because the SATATUS is still showing Provisioning. Then immediately in the very next step deletion of service is invoked and causes the failure

Running odo with args [odo delete -f dh-postgresql-apb --app uphmxpo --project ajdvqxarqf]
[odo]  ✗  Component dh-postgresql-apb does not exist in application uphmxpo

what is odd is that the delete command uses ajdvqxarqf but the error message mentiones uphmxpo

@amitkrout
Copy link
Contributor

It seems the service is not created properly because the SATATUS is still showing Provisioning. Then immediately in the very next step deletion of service is invoked and causes the failure

Running odo with args [odo delete -f dh-postgresql-apb --app uphmxpo --project ajdvqxarqf]
[odo]  ✗  Component dh-postgresql-apb does not exist in application uphmxpo

what is odd is that the delete command uses ajdvqxarqf but the error message mentiones uphmxpo

uphmxpo is the app name

@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

uphmxpo is the app name

oh, my bad 😇, sorry

but still, the deletion should work regardless of the status

@amitkrout
Copy link
Contributor

but still, the deletion should work regardless of the status

Ideally it should, but no idea why its not working. IMO the deletion code need some amount of investigation. I will create a separate followup issue.

@kadel kadel added priority/critical-urgent and removed flake Categorizes issue or PR as related to a flaky test. labels Aug 14, 2019
@kadel kadel changed the title test flake: wrong project name used Failing service test Aug 14, 2019
@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

This is blocking all PR right now :-(

@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

▶ odo service list --app uphmxpo --project ajdvqxarqf
NAME                  TYPE                  STATUS
dh-postgresql-apb     dh-postgresql-apb     ProvisionedAndBound

github.com/openshift/odo  master ✔                                                                                                                                                                 18h10m
▶ odo delete -f dh-postgresql-apb --app uphmxpo --project ajdvqxarqf
 ✗  Component dh-postgresql-apb does not exist in application uphmxpo

@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

LOL, the test is not deleting service, but it is trying to delete component !!!
How the hell this got in?

@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

This got in with #2001

@amitkrout
Copy link
Contributor

This got in with #2001

Ahh

@kadel
Copy link
Member Author

kadel commented Aug 14, 2019

Screenshot 2019-08-14 at 13 25 29

@amitkrout Can you please look into how this happened. It looks like that travis tests are not set as required :-(

@kadel kadel changed the title Failing service test PR got merged even when TravisCI test were failing Aug 14, 2019
@amitkrout
Copy link
Contributor

This is very unfortunate. Will make it required then. cc_ @mohammedzee1000

@amitkrout
Copy link
Contributor

amitkrout commented Aug 14, 2019

Screenshot 2019-08-14 at 13 25 29

@amitkrout Can you please look into how this happened. It looks like that travis tests are not set as required :-(

I am surprise, why i was not ping by the bot in this pr

@mohammedzee1000
Copy link
Contributor

Travis was removed from required due to incompatibility with prow. I will see if this was sorted. Otherwise, we cant make it as required.
I would say until then, make sure to exercise caution with satisfying the merge conditions

@amitkrout
Copy link
Contributor

amitkrout commented Aug 21, 2019

Travis was removed from required due to incompatibility with prow. I will see if this was sorted. Otherwise, we cant make it as required.

As per platform team they don't expect to support Travis soon

@amitkrout
Copy link
Contributor

amitkrout commented Aug 22, 2019

Travis was removed from required due to incompatibility with prow. I will see if this was sorted. Otherwise, we cant make it as required.

As per platform team they don't expect to support Travis soon
As per the slack thread discussion there is no ETA for travis support. So reducing the priority cc_

@kadel
/priority medium

@openshift-ci-robot openshift-ci-robot added the priority/Medium Nice to have issue. Getting it done before priority changes would be great. label Aug 22, 2019
@amitkrout
Copy link
Contributor

Travis was removed from required due to incompatibility with prow. I will see if this was sorted. Otherwise, we cant make it as required.
I would say until then, make sure to exercise caution with satisfying the merge conditions

@girishramnani @dharmit @cdrage @mik-dass ^

@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 24, 2020
@kadel
Copy link
Member Author

kadel commented Apr 28, 2020

/remove-lifecycle rotten

This is starting to cause a lot of problems :-(

We can create a temporary workaround.
Let's create a job on OpenShift CI, that will check Travis-CI status for given PR.
The job should have all information that is needed (PR number, repo). It should be just one simple API call to github status API endpoint.

/priority high
/remove-priority medium

@openshift-ci-robot openshift-ci-robot added priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/Medium Nice to have issue. Getting it done before priority changes would be great. labels Apr 28, 2020
@amitkrout
Copy link
Contributor

/remove-lifecycle rotten

This is starting to cause a lot of problems :-(

We can create a temporary workaround.
Let's create a job on OpenShift CI, that will check Travis-CI status for given PR.
The job should have all information that is needed (PR number, repo). It should be just one simple API call to github status API endpoint.

/priority high
/remove-priority medium

To get the travis login, status, PR number we need shared secret and ofcourse env variable which is not supported by the template based test. @prietyc123 is working on the migration part in #3270 which supports secret sharing and exposing of environment variable that we are looking for.

So keeping it as For consideration

@dharmit dharmit added priority/Medium Nice to have issue. Getting it done before priority changes would be great. and removed priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). labels Jun 22, 2020
@kadel
Copy link
Member Author

kadel commented Jun 22, 2020

Blocked on #3270

/status blocked

@kadel
Copy link
Member Author

kadel commented Aug 7, 2020

/remove-status blocked
#3270 was merged

@amitkrout
Copy link
Contributor

amitkrout commented Aug 20, 2020

Unfortunately GitHub checks (the new GH test API) are not visible to Prow, so the merge bot cannot be
configured to require them for merging. I think it should be possible to configure that the check is needed for 
merges in branch protection somehow, then the bot will not be able to merge unless that check is green.

I tried it. Seems it's working but there are consistent /retest trigger is happening. AFAIU /retest will only rerun Prow's failed jobs, so there are no actual unnecessary retests

@kadel
Copy link
Member Author

kadel commented Aug 21, 2020

@amitkrout not sure I understand :-(
So if you configure branch protection for the master branch to make Travis-CI tests required it works or Prow ends in endless /retest loop?

@amitkrout
Copy link
Contributor

@amitkrout not sure I understand :-(
So if you configure branch protection for the master branch to make Travis-CI tests required it works. Prow ends in endless /retest loop?

yes, is works. Prow sense the travis failure but prow goes into endless /retest loop even after the successful prow CI run. Prow bot merges the pr only when travis job goes green after a job restart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/refactoring Issues or PRs related to code refactoring area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering estimated-size/L (20-40) Rough sizing for Epics. About 2 sprints of work for a person. priority/Medium Nice to have issue. Getting it done before priority changes would be great.
Projects
Archived in project
8 participants