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

RHOAIENG-10827: feat(nbcs): update ose-oauth-proxy image digest reference from 4.8 to the latest 4.14 version #386

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Sep 18, 2024

Description

https://issues.redhat.com/browse/RHOAIENG-10827

How Has This Been Tested?

Wait for OCP-CI to build this and deploy it in some RHOAI.

  • quay.io/opendatahub/kubeflow-notebook-controller:pr-386
  • quay.io/opendatahub/odh-notebook-controller:pr-386

Need to also edit args in the odh-notebook-controller deployment because normally the oauth proxy digest arg comes from rhods operator.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@jiridanek jiridanek force-pushed the injected_oauth_imagepullpolicy_change_and_sha256_digest branch from e60ec47 to 636ff62 Compare September 18, 2024 15:26
@jiridanek
Copy link
Member Author

/cherrypick stable

@openshift-cherrypick-robot

@jiridanek: once the present PR merges, I will cherry-pick it on top of stable in a new PR and assign it to you.

In response to this:

/cherrypick stable

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-sigs/prow repository.

@shalberd
Copy link

shalberd commented Sep 18, 2024

yeah, that is what I am currently using as well on OCP 4.14.
However, pre-4.14 ose-oauth images did not pose any problems, I just made it good practice to override the default in the manifests. Good that default in golang code is now 4.14 as well.

@shalberd
Copy link

shalberd commented Sep 18, 2024

@jiridanek Does it make sense to leave imagePullPolicy at Always in

https://github.com/opendatahub-io/kubeflow/blob/v1.7-branch/components/odh-notebook-controller/controllers/notebook_webhook.go#L78

vs

corev1.PullIfNotPresent?

@@ -25,7 +25,7 @@ spec:
imagePullPolicy: Always
command:
- /manager
args: ["--oauth-proxy-image", "registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4bef31eb993feb6f1096b51b4876c65a6fb1f4401fee97fa4f4542b6b7c9bc46"]
args: ["--oauth-proxy-image", "registry.redhat.io/openshift4/ose-oauth-proxy@sha256:105307b602ac74649f868e1ea0aab7b8621ea1ecfd58ceca669dcf32f538798e"]
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, how did you get this sha? 🤔 When I check the PR for the Dashboard opendatahub-io/odh-dashboard#3216, it uses the following sha:

4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695

Also, when I tried to create an ImageStream on my OpenShift cluster, I can see following:

kind: ImageStreamTag
apiVersion: image.openshift.io/v1
metadata:
  name: 'example:v4.14'
  namespace: default
  uid: f41c7b88-c7e3-4e56-bc11-bc389053e033
  resourceVersion: '44113055'
  creationTimestamp: '2024-09-19T07:45:52Z'
tag:
  name: v4.14
  annotations: null
  from:
    kind: DockerImage
    name: 'registry.redhat.io/openshift4/ose-oauth-proxy:v4.14'
  generation: 2
  importPolicy:
    scheduled: true
    importMode: PreserveOriginal
  referencePolicy:
    type: Source
generation: 2
lookupPolicy:
  local: false
image:
  metadata:
    name: 'sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695'
    uid: bcfc4de2-b22b-49fb-9d1e-c2d65ddb4201
    resourceVersion: '44113054'
    creationTimestamp: '2024-09-19T07:45:52Z'
  dockerImageReference: 'registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695'
  dockerImageMetadata:
    kind: DockerImage
    apiVersion: image.openshift.io/1.0
    Id: 'sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695'
    Created: '2024-09-19T07:45:50Z'
    ContainerConfig: {}
  dockerImageMetadataVersion: '1.0'
  dockerImageManifestMediaType: application/vnd.docker.distribution.manifest.list.v2+json
  dockerImageManifests:
    - digest: 'sha256:105307b602ac74649f868e1ea0aab7b8621ea1ecfd58ceca669dcf32f538798e'
      mediaType: application/vnd.docker.distribution.manifest.v2+json
      manifestSize: 760
      architecture: amd64
      os: linux
    - digest: 'sha256:c55ea5f29f59eccbb57d5606d84ab53d9987b0226e1e93d76aefa22bc2b82d0b'
      mediaType: application/vnd.docker.distribution.manifest.v2+json
      manifestSize: 760
      architecture: arm64
      os: linux
    - digest: 'sha256:0eb617ef510f990b11a3144077bda3bc96bdd2a9fd19bff9d1f3d11cab243c79'
      mediaType: application/vnd.docker.distribution.manifest.v2+json
      manifestSize: 760
      architecture: ppc64le
      os: linux
    - digest: 'sha256:1cff8c85d0befc48805d4339913fd0e7f02896c1c0ad313e3e4ec2e001ccecb9'
      mediaType: application/vnd.docker.distribution.manifest.v2+json
      manifestSize: 760
      architecture: s390x
      os: linux

My guess is you used some manifests sha instead? Or am I missing something? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I used image sha, what dashboard pr uses is manifests sha.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks Jiri, eventually they changed it to platform agnostic opendatahub-io/odh-dashboard@6d95c02 and use back the manifest digest

Copy link

@shalberd shalberd Sep 19, 2024

Choose a reason for hiding this comment

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

correct. For a given tag, like v4.14, the digest changes every now and then, I think there is a stream kind of regular build process behind it.
Platform agnostic is what "manifest list digest" refers to
Google cloud docs explain it like this:
"The optional image index, sometimes referred to as the manifest list, refers to one or more image manifests. The reference is the digest of the image manifest. An image index is useful when you produce multiple related images for different platforms, such as amd64 and arm64 architectures."
Using manifest list digest is a good idea, nice to see the dashboard folks are using that one, too.
I always use manifest list digests and basically let the build and pull infrastructure pick the right architecture from the list in the manifest, either implicitly or with an architecture argument in podman or docker.

Screenshot 2024-09-19 at 11 07 22 Screenshot 2024-09-19 at 11 07 34

Currently, the manifest list digest from the build 8 days ago is
sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695

Copy link
Member

@jstourac jstourac left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link

openshift-ci bot commented Sep 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jstourac

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

@jstourac
Copy link
Member

Note: @jiridanek once this is merged, we shall cherry-pick this to stable branch (as you already tried), but eventually also into the 1.9 branch.

@jiridanek
Copy link
Member Author

jiridanek commented Sep 19, 2024

Works for me, the oauth image changed from registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4bef31eb993feb6f1096b51b4876c65a6fb1f4401fee97fa4f4542b6b7c9bc46 to registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695 and I was able to auth into my workbench.

Imo this is good enough to go into odh-nightly for more testing, but I'd leave that decision on the others. I will put that cherrypick to stable on hold, pending grooming and planning on the linked Jira issue.

@jiridanek
Copy link
Member Author

/cherrypick v1.9-branch

@openshift-cherrypick-robot

@jiridanek: once the present PR merges, I will cherry-pick it on top of v1.9-branch in a new PR and assign it to you.

In response to this:

/cherrypick v1.9-branch

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-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 703c348 into opendatahub-io:v1.7-branch Sep 19, 2024
12 checks passed
@jiridanek jiridanek deleted the injected_oauth_imagepullpolicy_change_and_sha256_digest branch September 19, 2024 11:15
@openshift-cherrypick-robot

@jiridanek: new pull request created: #387

In response to this:

/cherrypick stable

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-sigs/prow repository.

@openshift-cherrypick-robot

@jiridanek: new pull request created: #388

In response to this:

/cherrypick v1.9-branch

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-sigs/prow repository.

@atheo89
Copy link
Member

atheo89 commented Sep 30, 2024

/cherrypick main

@openshift-cherrypick-robot

@atheo89: new pull request created: #400

In response to this:

/cherrypick main

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants