Skip to content

Conversation

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Mar 31, 2019

To ensure the payload is reproducible, we will include images for both cli
and installer commands for non linux platforms that allow users to get the
correct tools to install the release image. Since the installer binary is
not small and we don't want to take the cost to pull it on each node when
in use from hive, create a new image installer-artifacts that layers on top
of installer (inheriting the default installer Linux binary) and places the
darwin binary into
/usr/share/openshift/mac/openshift-install. This directory structure is
kept deliberately simple for end users - if we do in the future need to
deal with multi-arch concerns we'll do that at a higher level and in
practice neither 32bit nor arm will be "supported" as part of the core
distro yet.

The dockerfile matches the desired final form from the release team
(Dockerfile.rhel) and will be used in both CI and publishing.

Used by openshift/origin#22439

To ensure the payload is reproducible, we will include images for
both cli and installer commands for non linux platforms that allow
users to get the correct tools to install the release image. Since
the installer binary is not small and we don't want to take the cost
to pull it on each node when in use from hive, create a new image
installer-artifacts that layers on top of installer (inheriting the
default installer Linux binary) and places the darwin binary into
/usr/share/openshift/mac/openshift-install. This directory structure
is kept deliberately simple for end users - if we do in the future
need to deal with multi-arch concerns we'll do that at a higher level
and in practice neither 32bit nor arm will be "supported" as part
of the core distro yet.

The dockerfile matches the desired final form from the release team
(Dockerfile.rhel) and will be used in both CI and publishing.
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 31, 2019
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2019
@smarterclayton
Copy link
Contributor Author

/assign @wking

I can't create the comet request due to an error but will do so on Monday

Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

I think it would also be nice to serve a cross-platform image so Linux users could pull the image and only get the Linux binary, macOS users could pull the image and only get the Darwin binary, etc. Then we'd have a third image that pulled those binaries out from the various manifests of the multi-platform image and sorted them into the GOARCH/GOOS structure for serving from the cluster. That wouldn't share layers anymore, but I expect Hive to only use the Linux manifest, and the console download Deployment to serve from the assembled, multi-platform image. Will Hive and the console download pods often be running on the same node, where sharing layers will be important?

RUN SKIP_GENERATION=y GOOS=darwin GOARCH=amd64 hack/build.sh

FROM registry.svc.ci.openshift.org/ocp/4.0:installer
COPY --from=builder /go/src/github.com/openshift/installer/bin/openshift-install /usr/share/openshift/mac/openshift-install
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather stick with /$GOARCH/GOOS/openshift-install, like openshift/console-operator#177. I see three major consumers of these images:

  • openshift-install itself, where using the Go names makes it easy for us to construct and spit out a download link for oc. That doesn't directly impact openshift-install, but I think we should use the same pattern for oc and openshift-install to minimize mental overhead.
  • The web console, which is free to provide whatever translation is sees fit when suggesting download locations (e.g. "mac" or "macOS" text linking to a darwin URL). This is the approach Terraform uses, with nifty logos, and I think that works pretty well.
  • External docs like https://cloud.openshift.com/clusters/install. It sounds like the plan is to keep these pointing into here or similar, but can't we drop some HTML in there to get something prettier than a raw Apache directory browser? Once the web-console folks have worked something up, it should be easy to create a form with relative links for the download binaries drop copies into each of those download directories as we publish them, right?

Copy link
Contributor Author

@smarterclayton smarterclayton Mar 31, 2019

Choose a reason for hiding this comment

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

I don't want the structure of this in the� image because "darwin" is user hostile for the majority of our users (as is the arch). This is primarily intended for consumers, not for our build infra, so we should bias towards them.

We have a small chance of ever supporting alternative architectures for Windows or Mac. We have a slightly larger chance of supporting linux alt arches (arm / power) but those are going to be special flows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at 177 I don't think that's the structure a human should see, but more of a machine actor (and I'm not sure what machine actor would use that instead of the cli image).

Copy link
Contributor Author

@smarterclayton smarterclayton Mar 31, 2019

Choose a reason for hiding this comment

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

External docs like https://cloud.openshift.com/clusters/install. It sounds like the plan is to keep these pointing into here or similar, but can't we drop some HTML in there to get something prettier than a raw Apache directory browser?

The plan is that that directory is going to be created by running oc adm release extract ... which creates a directory per release, and uses windows, linux, mac. Right now the discussed structure was:

root/
  latest-openshift-client-mac.tar.gz -> symlink to 4.0.5/openshift-install-mac-4.0.5.tar.gz
  latest -> symlink to a dir like 4.0.5/
  4.0.6/
    README.md
    LICENSE
    openshift-install-linux-4.0.6.tar.gz
    openshift-install-mac-4.0.6.tar.gz
    openshift-client-linux-4.0.6.tar.gz
    openshift-client-mac-4.0.6.tar.gz
    openshift-client-windows-4.0.6.zip
  4.0.5/
    ...
  4.0.4/
    ...

Copy link
Contributor Author

@smarterclayton smarterclayton Apr 1, 2019

Choose a reason for hiding this comment

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

After thinking about this a bit more, if we can make the argument that the location in the image will never be viewed by non-developers, I could buy /usr/share/openshift/GOOS/GOARCH/BINARY_NAME. But only if we don't expect anyone outside our team to ever see that directory.

Copy link
Member

Choose a reason for hiding this comment

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

But only if we don't expect anyone outside our team to ever see that directory.

Yeah, I think "drop an index.html in there (possibly instead of the README.md?) so nobody has to look at the names (and possible subdir structure) ;).

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Mar 31, 2019

I think it would also be nice to serve a cross-platform image so Linux users could pull the image and only get the Linux binary, macOS users could pull the image and only get the Darwin binary, etc. Then we'd have a third image that pulled those binaries out from the various manifests of the multi-platform image and sorted them into the GOARCH/GOOS structure for serving from the cluster. That wouldn't share layers anymore, but I expect Hive to only use the Linux manifest, and the console download Deployment to serve from the assembled, multi-platform image. Will Hive and the console download pods often be running on the same node, where sharing layers will be important?

Why would we need that? The serving image would be redundant to the existing images.

Sharing layers is a pretty big deal - adding an extra half gig to the mirror payload just for some binaries people use a very small percentage of time isn't a good tradeoff. We don't need zips and tars because we have images, which are zips and tars.

@wking
Copy link
Member

wking commented Apr 1, 2019

I still think we want a multi-platform image where the various targets get first-class, can-run-with-a-container-runtime representation. But since this is only intended to support oc adm release extract ... and ART's download mirrors, I'm fine with whatever keeps the ball rolling now, and we can revisit later.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 1, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [smarterclayton,wking]

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

@openshift-bot
Copy link
Contributor

/retest

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

@smarterclayton
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit d89fb5f into openshift:master Apr 1, 2019
@DanyC97
Copy link
Contributor

DanyC97 commented Jun 7, 2019

this is pretty awesome @wking @smarterclayton , thanks Gents !

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants