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.

Sign up for GitHub

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

Jump to bottom

Removes the s2i flag from the create file #5023

Merged
merged 1 commit into from
Sep 21, 2021
Merged

Removes the s2i flag from the create file #5023

merged 1 commit into from
Sep 21, 2021

Conversation

mik-dass
Copy link
Contributor

@mik-dass mik-dass commented Aug 26, 2021 •
edited

What type of PR is this?

/kind cleanup

What does this PR do / why we need it:

It removes the s2i flag from the create command.

Which issue(s) this PR fixes:

Fixes part of #4932
Fixes #5012

PR acceptance criteria:

How to test changes / Special notes to the reviewer:

  • Tests should pass.

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. kind/cleanup labels Aug 26, 2021
@dharmit
Copy link
Member

dharmit commented Aug 26, 2021

Please check if this fixes #5012.

@mik-dass
Copy link
Contributor Author

mik-dass commented Sep 2, 2021 

Ginkgo ran 1 suite in 1m27.490986988s
Test Suite Passed
+ '[' true ']'
+ exit -1
{"component":"entrypoint","error":"wrapped process failed: exit status 255","file":"prow/entrypoint/run.go:80","func":"k8s.io/test-infra/prow/entrypoint.Options.Run","level":"error","msg":"Error executing test process","severity":"error","time":"2021-08-27T07:49:19Z"}
error: failed to execute wrapped command: exit status 255 
�[36mINFO�[0m[2021-08-27T07:49:29Z] Step integration-e2e-integration-e2e-steps failed after 37m50s. 
�[36mINFO�[0m[2021-08-27T07:49:29Z] Running step integration-e2e-gather-aws-console. 
�[36mINFO�[0m[2021-08-27T07:49:49Z] Step integration-e2e-gather-aws-console succeeded after 20s. 
�[36mINFO�[0m[2021-08-27T07:49:49Z] Running step integration-e2e-gather-must-gather. 
�[36mINFO�[0m[2021-08-27T07:52:39Z] Step integration-e2e-gather-must-gather succeeded after 2m50s. 
�[36mINFO�[0m[2021-08-27T07:52:39Z] Running step integration-e2e-gather-extra.   
�[36mINFO�[0m[2021-08-27T07:54:29Z] Step integration-e2e-gather-extra succeeded after 1m50s. 
�[36mINFO�[0m[2021-08-27T07:54:29Z] Running step integration-e2e-gather-audit-logs. 
�[36mINFO�[0m[2021-08-27T07:55:09Z] Step integration-e2e-gather-audit-logs succeeded after 40s. 
�[36mINFO�[0m[2021-08-27T07:55:09Z] Running step integration-e2e-ipi-deprovision-deprovision. 
�[36mINFO�[0m[2021-08-27T07:59:59Z] Step integration-e2e-ipi-deprovision-deprovision succeeded after 4m50s. 
�[36mINFO�[0m[2021-08-27T07:59:59Z] Releasing leases for test integration-e2e    
�[36mINFO�[0m[2021-08-27T07:59:59Z] Ran for 1h32m54s                             
�[31mERRO�[0m[2021-08-27T07:59:59Z] Some steps failed:                           
�[31mERRO�[0m[2021-08-27T07:59:59Z] 
  * could not run steps: step integration-e2e failed: "integration-e2e" test steps failed: "integration-e2e" pod "integration-e2e-integration-e2e-steps" failed: the pod ci-op-1rxvnxi0/integration-e2e-integration-e2e-steps failed after 37m41s (failed containers: test): ContainerFailed one or more containers exited

Container test exited with code 1, reason Error

/retest

@mik-dass mik-dass force-pushed the delete_s2i_cli_cmp branch 6 times, most recently from 6906b24 to 1b55784 Compare September 6, 2021 09:41
@prietyc123
Copy link
Contributor

@mik-dass Few Most of the scenarios in generic test file does not seem to be generic. For Ex https://github.com/openshift/odo/pull/5023/files?file-filters%5B%5D=.adoc&file-filters%5B%5D=.go&file-filters%5B%5D=.sh#diff-bb9e5f2fbc1b8edb31629193e23d6ec67b8963e0b730d9cb1f89305cd561a266L60-L130
Could you please fit those scenarios into respective file or remove if already exists

prietyc123
prietyc123 suggested changes Sep 6, 2021
View reviewed changes
Makefile
@@ -284,33 +256,11 @@ test-cmd-debug: ## Run odo debug command tests
$(RUN_GINKGO) $(GINKGO_FLAGS) -focus="odo debug command tests" tests/integration/
$(RUN_GINKGO) $(GINKGO_FLAGS_SERIAL) -focus="odo debug command serial tests" tests/integration/debug/

# Service, link and login/logout command tests are not the part of this test run
.PHONY: test-integration
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say only remove $(RUN_GINKGO) $(GINKGO_FLAGS_SERIAL) tests/integration/debug/ target because I can see some files are still targeting to tests/integration/ test suite. For ex - generic_test.go , cmd_perf_config_test.go ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@mik-dass
Copy link
Contributor Author

mik-dass commented Sep 6, 2021

@mik-dass Few Most of the scenarios in generic test file does not seem to be generic. For Ex https://github.com/openshift/odo/pull/5023/files?file-filters%5B%5D=.adoc&file-filters%5B%5D=.go&file-filters%5B%5D=.sh#diff-bb9e5f2fbc1b8edb31629193e23d6ec67b8963e0b730d9cb1f89305cd561a266L60-L130
Could you please fit those scenarios into respective file or remove if already exists

This should be done in a separate PR as this PR is already huge.

@mik-dass mik-dass changed the title [WIP] Removes the s2i flag from the create file Removes the s2i flag from the create file Sep 6, 2021
@openshift-ci openshift-ci bot 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 Sep 6, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Sep 12, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Sep 13, 2021
@mik-dass mik-dass force-pushed the delete_s2i_cli_cmp branch 4 times, most recently from eef4dc6 to 75a427c Compare September 14, 2021 10:16
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 15, 2021
@kadel
Copy link
Member

kadel commented Sep 21, 2021

/approve
/refresh

@openshift-ci
Copy link

openshift-ci bot commented Sep 21, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Sep 21, 2021
@openshift-bot
Copy link

/retest-required

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

1 similar comment
@openshift-bot
Copy link

/retest-required

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

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 21, 2021
@sonarcloud
Copy link

sonarcloud bot commented Sep 21, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
No Duplication information No Duplication information

@feloy
Copy link
Contributor

feloy commented Sep 21, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 21, 2021
@openshift-ci
Copy link

openshift-ci bot commented Sep 21, 2021 •
edited

@mik-dass: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/psi-k8s-ibmc-integration-e2e 75a427c link /test psi-k8s-ibmc-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.

@openshift-bot
Copy link

/retest-required

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

1 similar comment
@openshift-bot
Copy link

/retest-required

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

@openshift-merge-robot openshift-merge-robot merged commit 74fc779 into redhat-developer:main Sep 21, 2021
@kadel kadel mentioned this pull request Oct 15, 2021
Closed
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prietyc123 prietyc123 prietyc123 requested changes

@feloy feloy feloy approved these changes

@mohammedzee1000 mohammedzee1000 Awaiting requested review from mohammedzee1000

Assignees

@feloy feloy

@prietyc123 prietyc123

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. area/refactoring Issues or PRs related to code refactoring lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

odo returns misguided error message when running odo exec from a non-component directory
8 participants
@mik-dass @dharmit @prietyc123 @kadel @openshift-bot @feloy @rm3l @openshift-merge-robot