Bug 2009664: fix edit ksvc in git import flow #10255

debsmita1 · 2021-10-18T19:42:30Z

Fixes:
https://issues.redhat.com/browse/OCPBUGSM-35728

Root analysis:
The new revision failed to come up because it failed to fetch the image from the registry.

Solution description:

creating a new image stream only when the git url/image is changed
creating a new image stream when the serverless workload (which is created via internal registry flow -> Deployment -> Make serverless) is modified

GIF:

openshift-ci · 2021-10-18T19:42:37Z

@debsmita1: This pull request references Bugzilla bug 2009664, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sanketpathak

In response to this:

Bug 2009664: fix edit ksvc in git import flow

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.

debsmita1 · 2021-10-18T19:42:38Z

/kind bug

debsmita1 · 2021-10-18T19:43:30Z

/cc @karthikjeeyar
/cc @divyanshiGupta
/assign @invincibleJai

nemesis09 · 2021-10-21T10:31:01Z

/retest

nemesis09 · 2021-10-21T13:21:26Z

e2e tests seem to be failing.
I tried to run it locally but it seems the PR is missing a fix from #10254

debsmita1 · 2021-10-21T14:30:04Z

e2e tests seem to be failing. I tried to run it locally but it seems the PR is missing a fix from #10254

@nemesis09 Rebased. PTAL

invincibleJai · 2021-10-21T14:56:35Z

I think we need to create a new imagestream only if the image is impacted i.e if url is changed otherwise for any other change in ksvc like change scaling options, labels etc should not create any new image steam. And this behaviour is in deploy image as well so it'll be nice to get the fix there as well as we are created extra imagestreams which can be avoided.

karthikjeeyar · 2021-10-21T16:19:35Z

frontend/packages/dev-console/src/components/import/import-submit-utils.ts

+          referencePolicy: { type: 'Local' },
+        },
+      ],
+    },
  };
  const imageStream = mergeData(originalImageStream, newImageStream);
  return verb === 'update'


Verb is always set to true even in edit flow 🤔 , so Imagestreams are getting created everytime.

openshift-ci · 2021-10-26T12:42:29Z

@debsmita1: This pull request references Bugzilla bug 2009664, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sanketpathak

In response to this:

Bug 2009664: fix edit ksvc in git import flow

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.

karthikjeeyar · 2021-10-26T13:48:37Z

@debsmita1 I verified locally, By updating the git URL the revisions are now coming up successfully, however a new imagestream is getting created in case of updating just the Scaling Options under Advanced options in GIT import flow. Should we handle the imagestream creation incase of just updating scaling options?

Note: Updating the scaling options is the Deploy Image flow doesn't create new imagestream (expected) and revisions are coming up there as well.

Edit: Make Serverless option creates a new Imagestream for the new KSVC workload and that is expected. Git flow, Deploy Image and Make serverless options work fine.

/lgtm

karthikjeeyar · 2021-10-26T13:59:50Z

/lgtm

invincibleJai · 2021-10-29T11:59:42Z

frontend/packages/dev-console/src/components/import/import-submit-utils.ts

+  const originalRepository =
+    appResources?.buildConfig?.data?.spec?.source?.git?.uri ??
+    appResources?.pipeline?.data?.spec?.params?.find((param) => param?.name === 'GIT_REPO')
+      ?.default ??
+    null;


can we have as below as no need to explicitly return null i think

Suggested change

const originalRepository =

appResources?.buildConfig?.data?.spec?.source?.git?.uri ??

appResources?.pipeline?.data?.spec?.params?.find((param) => param?.name === 'GIT_REPO')

?.default ??

null;

const originalRepository =

appResources?.buildConfig?.data?.spec?.source?.git?.uri ??

appResources?.pipeline?.data?.spec?.params?.find((param) => param?.name === 'GIT_REPO')

?.default;

invincibleJai · 2021-10-29T13:58:41Z

@debsmita1 on edit for ksvc if I change git repo for git import flow or change image for container image flow then revision gets created and it does come up but I still see it's not serving the content from the new image. We need to look into this.

debsmita1 · 2021-10-29T17:05:18Z

@debsmita1 on edit for ksvc if I change git repo for git import flow or change image for container image flow then revision gets created and it does come up but I still see it's not serving the content from the new image. We need to look into this.

@invincibleJai tried this out in both the flows

invincibleJai · 2021-11-02T11:05:00Z

@debsmita1 I verified again and could not see above issue and overall looks good, I overserved one issue with the edit of ksvc (which is created via make Serverless flow)

edit of ksvc (which is created via make Serverless flow for external image)
creation of ksvc via make Serverless flow for internal image fails.

openshift-ci · 2021-12-08T11:06:21Z

@debsmita1: This pull request references Bugzilla bug 2009664, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sanketpathak

In response to this:

Bug 2009664: fix edit ksvc in git import flow

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.

debsmita1 · 2021-12-08T11:06:35Z

@debsmita1 I verified again and could not see above issue and overall looks good, I overserved one issue with the edit of ksvc (which is created via make Serverless flow)

edit of ksvc (which is created via make Serverless flow for external image)

creation of ksvc via make Serverless flow for internal image fails.

Fixed! PTAL

rohitkrai03

/approve

karthikjeeyar · 2021-12-10T13:43:31Z

@debsmita1 I tested the PR locally, Revisions are coming up properly but I observed a strange thing. When I edit the url from a nodejs repo to spring boot repo url, the revisions come up properly but the image the ksvc has is still pointing to the nodejs (old image). After a long time it automatically resets to newly edited image (spring boot). Not sure if this is the default knative behavior.
cc: @invincibleJai

invincibleJai · 2021-12-14T10:58:01Z

@debsmita1 I tested the PR locally, Revisions are coming up properly but I observed a strange thing. When I edit the url from a nodejs repo to spring boot repo url, the revisions come up properly but the image the ksvc has is still pointing to the nodejs (old image). After a long time it automatically resets to newly edited image (spring boot). Not sure if this is the default knative behavior. cc: @invincibleJai

@karthikjeeyar yeah this is expected behaviour as if we make a change in git import flow with edit flow then first build (S2I) happens, then image steam get updated and finally serverless pick it up so it takes a while. It will be quick in case of container image flow.

karthikjeeyar · 2021-12-14T11:01:44Z

@invincibleJai Yes, Container image flow is quick in updating to a new image, only git import took some time. Thanks for clarifying it.

/lgtm

openshift-ci · 2021-12-14T11:02:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: debsmita1, karthikjeeyar, rohitkrai03

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~~ [rohitkrai03]
~~frontend/packages/knative-plugin/OWNERS~~ [rohitkrai03]

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

openshift-bot · 2021-12-14T13:41:44Z

/retest-required

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

openshift-bot · 2021-12-14T13:54:45Z

/retest-required

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

openshift-ci · 2021-12-14T18:51:02Z

@debsmita1: All pull requests linked via external trackers have merged:

openshift/console#10255

Bugzilla bug 2009664 has been moved to the MODIFIED state.

In response to this:

Bug 2009664: fix edit ksvc in git import flow

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 · 2021-12-14T18:51:29Z

@debsmita1: all tests passed!

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-ci bot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Oct 18, 2021

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Oct 18, 2021

openshift-ci bot requested a review from sanketpathak October 18, 2021 19:42

openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 18, 2021

openshift-ci bot requested review from invincibleJai and nemesis09 October 18, 2021 19:42

openshift-ci bot added the component/dev-console Related to dev-console label Oct 18, 2021

openshift-ci bot assigned invincibleJai Oct 18, 2021

openshift-ci bot requested review from divyanshiGupta and karthikjeeyar October 18, 2021 19:43

debsmita1 force-pushed the ksvc-edit branch from e231203 to e59e7ba Compare October 21, 2021 13:36

karthikjeeyar reviewed Oct 21, 2021

View reviewed changes

debsmita1 force-pushed the ksvc-edit branch from e59e7ba to 29e81c1 Compare October 26, 2021 12:38

openshift-ci bot added the component/knative Related to knative-plugin label Oct 26, 2021

debsmita1 force-pushed the ksvc-edit branch from 29e81c1 to 7745d6a Compare October 26, 2021 12:40

debsmita1 requested a review from karthikjeeyar October 26, 2021 13:07

openshift-ci bot assigned karthikjeeyar Oct 26, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2021

invincibleJai reviewed Oct 29, 2021

View reviewed changes

debsmita1 force-pushed the ksvc-edit branch from 7745d6a to f10c25b Compare October 29, 2021 12:17

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2021

debsmita1 requested a review from invincibleJai October 29, 2021 12:18

fix edit ksvc in git import flow

3b6e13b

debsmita1 force-pushed the ksvc-edit branch from f10c25b to 3b6e13b Compare December 8, 2021 11:05

rohitkrai03 approved these changes Dec 8, 2021

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 14, 2021

openshift-merge-robot merged commit 13f9c7b into openshift:master Dec 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 2009664: fix edit ksvc in git import flow #10255

Bug 2009664: fix edit ksvc in git import flow #10255

debsmita1 commented Oct 18, 2021 •

edited

Loading

openshift-ci bot commented Oct 18, 2021

debsmita1 commented Oct 18, 2021

debsmita1 commented Oct 18, 2021

nemesis09 commented Oct 21, 2021

nemesis09 commented Oct 21, 2021

debsmita1 commented Oct 21, 2021

invincibleJai commented Oct 21, 2021 •

edited

Loading

karthikjeeyar Oct 21, 2021

openshift-ci bot commented Oct 26, 2021

karthikjeeyar commented Oct 26, 2021 •

edited

Loading

karthikjeeyar commented Oct 26, 2021

invincibleJai Oct 29, 2021

invincibleJai commented Oct 29, 2021 •

edited

Loading

debsmita1 commented Oct 29, 2021

invincibleJai commented Nov 2, 2021

openshift-ci bot commented Dec 8, 2021

debsmita1 commented Dec 8, 2021

rohitkrai03 left a comment

karthikjeeyar commented Dec 10, 2021 •

edited

Loading

invincibleJai commented Dec 14, 2021

karthikjeeyar commented Dec 14, 2021

openshift-ci bot commented Dec 14, 2021

openshift-bot commented Dec 14, 2021

openshift-bot commented Dec 14, 2021

openshift-ci bot commented Dec 14, 2021

openshift-ci bot commented Dec 14, 2021

Bug 2009664: fix edit ksvc in git import flow #10255

Bug 2009664: fix edit ksvc in git import flow #10255

Conversation

debsmita1 commented Oct 18, 2021 • edited Loading

openshift-ci bot commented Oct 18, 2021

debsmita1 commented Oct 18, 2021

debsmita1 commented Oct 18, 2021

nemesis09 commented Oct 21, 2021

nemesis09 commented Oct 21, 2021

debsmita1 commented Oct 21, 2021

invincibleJai commented Oct 21, 2021 • edited Loading

karthikjeeyar Oct 21, 2021

Choose a reason for hiding this comment

openshift-ci bot commented Oct 26, 2021

karthikjeeyar commented Oct 26, 2021 • edited Loading

karthikjeeyar commented Oct 26, 2021

invincibleJai Oct 29, 2021

Choose a reason for hiding this comment

invincibleJai commented Oct 29, 2021 • edited Loading

debsmita1 commented Oct 29, 2021

invincibleJai commented Nov 2, 2021

openshift-ci bot commented Dec 8, 2021

debsmita1 commented Dec 8, 2021

rohitkrai03 left a comment

Choose a reason for hiding this comment

karthikjeeyar commented Dec 10, 2021 • edited Loading

invincibleJai commented Dec 14, 2021

karthikjeeyar commented Dec 14, 2021

openshift-ci bot commented Dec 14, 2021

openshift-bot commented Dec 14, 2021

openshift-bot commented Dec 14, 2021

openshift-ci bot commented Dec 14, 2021

openshift-ci bot commented Dec 14, 2021

debsmita1 commented Oct 18, 2021 •

edited

Loading

invincibleJai commented Oct 21, 2021 •

edited

Loading

karthikjeeyar commented Oct 26, 2021 •

edited

Loading

invincibleJai commented Oct 29, 2021 •

edited

Loading

karthikjeeyar commented Dec 10, 2021 •

edited

Loading