Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry #2868

divyanshiGupta · 2019-09-30T12:20:36Z

Jira Issue - https://jira.coreos.com/browse/ODC-1911

Refactored import-submit-utis and deployImage-submit-utils

Extracted common code and made it shareable for both the forms
Added container resources to deploymentConfig in createDeploymentConfig of deployImage-submit-utils

To Do-

Unit tests for shared-submit-utils

divyanshiGupta · 2019-09-30T13:59:44Z

/test e2e-aws-console-olm

divyanshiGupta · 2019-09-30T14:14:16Z

/assign @rohitkrai03

christianvogt · 2019-10-02T17:55:55Z

frontend/packages/dev-console/src/utils/__tests__/shared-submit-utils.spec.ts

+import { createService, createRoute } from '../shared-submit-utils';
+import { GitImportFormData, DeployImageFormData } from '../../components/import/import-types';
+
+describe('Shared submit utils', () => {


Probably worth creating some snapshot tests here so that the entire output can be tested fully.

invincibleJai · 2019-10-03T06:57:07Z

@divyanshiGupta do you have any sample image to test which would have multiple ports exposed?

invincibleJai

I have verified locally and seems to work as expected.

invincibleJai · 2019-10-03T11:22:11Z

frontend/packages/dev-console/src/utils/__tests__/shared-submit-utils.spec.ts

+      const mockData: GitImportFormData = _.cloneDeep(mockFormData);
+      const mockDeployImageData: DeployImageFormData = _.cloneDeep(mockDeployImageFormData);
+      const PORT1 = { containerPort: 8081, protocol: 'TCP' };
+      let serviceObj;


do we know type of serviceObj? i think K8sResourceKind should work

So in K8sResourceKind podLabels is of type Selector but from getPodLabels we are returning a simple object with the required label values and not a Selector object. Hence if I assign K8sResourceKind it gives error. I tried to add a Selector Object with the required label values but while submitting the form it showed some error. That is why I have added any for now in the submit methods also for the resource object.

invincibleJai · 2019-10-03T11:25:53Z

frontend/packages/dev-console/src/utils/__tests__/shared-submit-utils.spec.ts

+    it('should set correct target port in route object', () => {
+      const mockData: GitImportFormData = _.cloneDeep(mockFormData);
+      const mockDeployImageData: DeployImageFormData = _.cloneDeep(mockDeployImageFormData);
+      let routeObj;


same as above

invincibleJai · 2019-10-03T11:40:25Z

frontend/packages/dev-console/src/components/import/image-search/ImageSearch.tsx

@@ -62,6 +62,7 @@ const ImageSearch: React.FC = () => {
          setFieldValue('isi.status', status);
          setFieldValue('isi.ports', ports);
          setFieldValue('image.ports', ports);
+          setFieldValue('image.tag', tag);


Any specific reason for passing tags fetched from imagestream to image.tag here, can we use isi tag only for labels in create service/routes.

Ideally, both would be same but it would be good to have them separate isi tag would be one generated as part of imageStream and image tag would be one provided by user( we may need this later as of now we are not fetching user entered tags)

If you see the code of shared submit utils, I have made it usable for both forms. So there are some properties of git import form data that are not available in deploy image form data and vice versa, for such properties I am using get. Before I did, if image exists in form data then its tag will be used otherwise isi.tag will be used, but since image is present in both the form data this was not the right way to get the tag because if we are on deploy image form in this case tag will always be empty since we are not setting image.tag anywhere. That is why I had to set it. This image and isi thing needs to be refactored. I planned to do it in this PR but it turned out that it will need some investigation to refactor it and not break the code anywhere else as both are being used at different places.

invincibleJai · 2019-10-03T19:51:22Z

/lgtm

christianvogt · 2019-10-04T01:53:02Z

/approve

openshift-ci-robot · 2019-10-04T01:55:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, divyanshiGupta, invincibleJai

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:

~~frontend/packages/dev-console/OWNERS~~ [christianvogt]

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

christianvogt · 2019-11-05T15:22:36Z

/retitle Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry

christianvogt · 2019-11-05T15:25:23Z

/bugzilla refresh

openshift-ci-robot · 2019-11-05T15:26:55Z

@divyanshiGupta: All pull requests linked via external trackers have merged. Bugzilla bug 1760836 has been moved to the MODIFIED state.

In response to this:

Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry

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.

openshift-ci-robot · 2019-11-05T15:27:42Z

@christianvogt: Bugzilla bug 1760836 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state.

In response to this:

/bugzilla refresh

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.

openshift-ci-robot requested review from debsmita1 and invincibleJai September 30, 2019 12:20

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. component/dev-console Related to dev-console labels Sep 30, 2019

openshift-ci-robot assigned rohitkrai03 Sep 30, 2019

divyanshiGupta changed the title ~~Bug: Deploy image ignores target port drop down selection and always uses first entry~~ [WIP]Bug: Deploy image ignores target port drop down selection and always uses first entry Oct 1, 2019

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 1, 2019

divyanshiGupta changed the title ~~[WIP]Bug: Deploy image ignores target port drop down selection and always uses first entry~~ Bug: Deploy image ignores target port drop down selection and always uses first entry Oct 1, 2019

openshift-ci-robot removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 1, 2019

christianvogt reviewed Oct 2, 2019

View reviewed changes

invincibleJai reviewed Oct 3, 2019

View reviewed changes

divyanshiGupta added 3 commits October 4, 2019 00:58

refactor deployImage and import submit utils

29cd9dc

add unit tests for shared submit utils

b2fe1c9

add snapshot tests

da3eec9

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 3, 2019

openshift-ci-robot assigned invincibleJai Oct 3, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 3, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2019

openshift-merge-robot merged commit d9f98f8 into openshift:master Oct 4, 2019

spadgett added this to the v4.3 milestone Oct 4, 2019

openshift-ci-robot changed the title ~~Bug: Deploy image ignores target port drop down selection and always uses first entry~~ Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry Nov 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry #2868

Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry #2868

divyanshiGupta commented Sep 30, 2019 •

edited

Loading

divyanshiGupta commented Sep 30, 2019

divyanshiGupta commented Sep 30, 2019

christianvogt Oct 2, 2019

divyanshiGupta Oct 3, 2019

invincibleJai commented Oct 3, 2019

invincibleJai left a comment

invincibleJai Oct 3, 2019

divyanshiGupta Oct 3, 2019

invincibleJai Oct 3, 2019

invincibleJai Oct 3, 2019

divyanshiGupta Oct 3, 2019

invincibleJai commented Oct 3, 2019

christianvogt commented Oct 4, 2019

openshift-ci-robot commented Oct 4, 2019

christianvogt commented Nov 5, 2019

christianvogt commented Nov 5, 2019

openshift-ci-robot commented Nov 5, 2019

openshift-ci-robot commented Nov 5, 2019

Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry #2868

Bug 1760836: Deploy image ignores target port drop down selection and always uses first entry #2868

Conversation

divyanshiGupta commented Sep 30, 2019 • edited Loading

divyanshiGupta commented Sep 30, 2019

divyanshiGupta commented Sep 30, 2019

christianvogt Oct 2, 2019

Choose a reason for hiding this comment

divyanshiGupta Oct 3, 2019

Choose a reason for hiding this comment

invincibleJai commented Oct 3, 2019

invincibleJai left a comment

Choose a reason for hiding this comment

invincibleJai Oct 3, 2019

Choose a reason for hiding this comment

divyanshiGupta Oct 3, 2019

Choose a reason for hiding this comment

invincibleJai Oct 3, 2019

Choose a reason for hiding this comment

invincibleJai Oct 3, 2019

Choose a reason for hiding this comment

divyanshiGupta Oct 3, 2019

Choose a reason for hiding this comment

invincibleJai commented Oct 3, 2019

christianvogt commented Oct 4, 2019

openshift-ci-robot commented Oct 4, 2019

christianvogt commented Nov 5, 2019

christianvogt commented Nov 5, 2019

openshift-ci-robot commented Nov 5, 2019

openshift-ci-robot commented Nov 5, 2019

divyanshiGupta commented Sep 30, 2019 •

edited

Loading