Remove deprecated images and update tests #3587

amitkrout · 2020-07-16T06:54:29Z

What type of PR is this?
/kind cleanup
/kind deprecation
/kind documentation

What does does this PR do / why we need it:
Clean up
Which issue(s) this PR fixes:

Fixes need for redhat-developer/odo-init-image#64

PR acceptance criteria:

Unit test
Integration test : NA - Tests are running on all s2i images in e2e scenario.
Documentation

How to test changes / Special notes to the reviewer:
old unsupported images are removed

openshift-ci-robot · 2020-07-16T06:54:32Z

@amitkrout: The label(s) kind/deprecation cannot be applied, because the repository doesn't have them

In response to this:

What type of PR is this?
/kind cleanup
/kind deprecation
/kind documentation

What does does this PR do / why we need it:
Clean up
Which issue(s) this PR fixes:

Fixes need for redhat-developer/odo-init-image#64

PR acceptance criteria:

Unit test

Integration test

Documentation

How to test changes / Special notes to the reviewer:
old unsupported images are removed

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.

mik-dass · 2020-07-16T07:27:14Z

pkg/catalog/catalog.go

-
-		// older images which we should remove soon
-		"rhoar-nodejs/nodejs-8:latest",
-		"rhoar-nodejs/nodejs-10:latest",


this seems like a nodejs-10 image, should we remove this too?

Unfortunately i have removed it. Will add it back

mik-dass · 2020-07-16T07:27:54Z

pkg/catalog/catalog.go

-		"rhoar-nodejs/nodejs-8:latest",
-		"rhoar-nodejs/nodejs-10:latest",
-		"bucharestgold/centos7-s2i-nodejs:8.x",
-		"bucharestgold/centos7-s2i-nodejs:10.x",


same as https://github.com/openshift/odo/pull/3587/files?short_path=7bd4a92#r455565860

@mik-dass This image support has been removed in pr #3414, but unfortunately the reference is left behind in the catalog.go file

These are all deprecated images

girishramnani · 2020-07-16T07:46:20Z

/approve

openshift-ci-robot · 2020-07-16T07:46:38Z

[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:

~~OWNERS~~ [girishramnani]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

prietyc123 · 2020-07-21T05:31:25Z

pkg/catalog/catalog_test.go

@@ -315,6 +314,7 @@ func MockImageStream() *imagev1.ImageStream {
 		"12": "docker.io/rhscl/nodejs-12-rhel7:latest",
 		"10": "docker.io/rhscl/nodejs-10-rhel7:latest",
 		"8":  "docker.io/rhoar-nodejs/nodejs-8:latest",
+
 		// an unspported one


AFAIU "8": and "6": both are unsupported one then why not shift this comment one step up

prietyc123 · 2020-07-21T05:35:55Z

Everything looks good to me 👍

/lgtm

openshift-bot · 2020-07-21T07:48:40Z

/retest

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

amitkrout · 2020-07-21T07:58:45Z

We have a genuine travis failure https://travis-ci.com/github/openshift/odo/jobs/363165202#L574
/hold

codecov · 2020-07-21T08:06:02Z

Codecov Report

Merging #3587 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3587      +/-   ##
==========================================
- Coverage   45.73%   45.69%   -0.04%     
==========================================
  Files         122      122              
  Lines       12237    12230       -7     
==========================================
- Hits         5596     5589       -7     
  Misses       6090     6090              
  Partials      551      551

Impacted Files	Coverage Δ
pkg/catalog/catalog.go	`53.74% <ø> (-1.04%)`	⬇️

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 cb129c8...723c99d. Read the comment docs.

devang-gaur · 2020-07-25T09:02:26Z

pkg/catalog/catalog_test.go

 		// an unspported one
+		"8": "docker.io/rhoar-nodejs/nodejs-8:latest",
 		"6": "docker.io/rhoar-nodejs/nodejs-6:latest",


Why not remove nodejs-6 image as well?

Kept it here just for UTs validation step on more values.

Validation done here - https://github.com/openshift/odo/pull/3587/files#diff-1dfbb1796e8d3ca31e26fe9d003bb488R168

devang-gaur · 2020-07-25T09:32:34Z

tests/integration/component.go

@@ -583,6 +583,12 @@ func componentTests(args ...string) {
 			output := helper.CmdShouldPass("odo", append(args, "create", "nodejs:latest", componentName, "--app", appName, "--project", project, "--context", context)...)
 			Expect(output).NotTo(ContainSubstring("Warning"))
 		})
+


if there is java specific code involved as well, you should change the PR description. I see the supported-java.json file being added as well. will be a good practice.

Make sense 👍

I could have split it into two prs but currently it is well organized so i don't want to split again. Yes, i will update the pr title.

amitkrout · 2020-07-25T14:38:18Z

/hold cancel

* Remove deprecated nodejs 8 images * Update README * Updated README * Addressed review comments * Fixing unit test failure * Addressed review comment * Fixing travis failure * Checking oc version * Applying supported image imagestream * Applying supported image imagestream * Added supported is * Debugging * Cleanup * Updated test doc * Cleanup

openshift-ci-robot added kind/cleanup labels Jul 16, 2020

openshift-ci-robot requested review from dharmit and GeekArthur July 16, 2020 06:54

mik-dass reviewed Jul 16, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jul 16, 2020

amitkrout force-pushed the removeDeprecatedNodejs8Images branch 2 times, most recently from aea86ab to 0989465 Compare July 21, 2020 03:01

prietyc123 suggested changes Jul 21, 2020

View reviewed changes

openshift-ci-robot assigned prietyc123 Jul 21, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 21, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Jul 21, 2020

amitkrout force-pushed the removeDeprecatedNodejs8Images branch from 1a7a256 to 306ec7d Compare July 22, 2020 07:03

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 22, 2020

amitkrout force-pushed the removeDeprecatedNodejs8Images branch 3 times, most recently from fefe5ad to 1c30cbc Compare July 24, 2020 14:47

amitkrout added 5 commits July 25, 2020 11:07

Remove deprecated nodejs 8 images

2fec272

Update README

6c73a75

Updated README

47621e7

Addressed review comments

0694976

Fixing unit test failure

eeb7c38

amitkrout added 6 commits July 25, 2020 11:07

Addressed review comment

a7d2383

Fixing travis failure

902ed1e

Checking oc version

918793c

Applying supported image imagestream

15519d3

Applying supported image imagestream

04c2da4

Added supported is

4a331d2

amitkrout force-pushed the removeDeprecatedNodejs8Images branch from b08528c to 4a331d2 Compare July 25, 2020 06:31

amitkrout added 2 commits July 25, 2020 12:52

Debugging

8bbdad0

Cleanup

64128a2

devang-gaur suggested changes Jul 25, 2020

View reviewed changes

openshift-ci-robot assigned devang-gaur Jul 25, 2020

amitkrout added 2 commits July 25, 2020 15:16

Updated test doc

9d30974

Cleanup

723c99d

amitkrout changed the title ~~Remove deprecated nodejs 8 images~~ Remove deprecated images and update tests Jul 25, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Jul 25, 2020

devang-gaur approved these changes Jul 26, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 26, 2020

openshift-merge-robot merged commit b3cf595 into redhat-developer:master Jul 27, 2020

amitkrout mentioned this pull request Jul 27, 2020

Remove deprecated Nodejs images redhat-developer/odo-init-image#64

Closed

rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove deprecated images and update tests #3587

Remove deprecated images and update tests #3587

amitkrout commented Jul 16, 2020 •

edited

Loading

openshift-ci-robot commented Jul 16, 2020

mik-dass Jul 16, 2020

amitkrout Jul 16, 2020

mik-dass Jul 16, 2020

amitkrout Jul 16, 2020

girishramnani Jul 16, 2020

girishramnani commented Jul 16, 2020

openshift-ci-robot commented Jul 16, 2020

prietyc123 Jul 21, 2020 •

edited

Loading

prietyc123 commented Jul 21, 2020

openshift-bot commented Jul 21, 2020

amitkrout commented Jul 21, 2020 •

edited

Loading

codecov bot commented Jul 21, 2020 •

edited

Loading

devang-gaur Jul 25, 2020

amitkrout Jul 25, 2020

devang-gaur Jul 26, 2020

devang-gaur Jul 25, 2020

amitkrout Jul 25, 2020

amitkrout Jul 25, 2020

amitkrout commented Jul 25, 2020

Remove deprecated images and update tests #3587

Remove deprecated images and update tests #3587

Conversation

amitkrout commented Jul 16, 2020 • edited Loading

openshift-ci-robot commented Jul 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

girishramnani commented Jul 16, 2020

openshift-ci-robot commented Jul 16, 2020

prietyc123 Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

prietyc123 commented Jul 21, 2020

openshift-bot commented Jul 21, 2020

amitkrout commented Jul 21, 2020 • edited Loading

codecov bot commented Jul 21, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitkrout commented Jul 25, 2020

amitkrout commented Jul 16, 2020 •

edited

Loading

prietyc123 Jul 21, 2020 •

edited

Loading

amitkrout commented Jul 21, 2020 •

edited

Loading

codecov bot commented Jul 21, 2020 •

edited

Loading