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

Split up the openshift image content to reduce total size. #19509

Merged
merged 26 commits into from
May 2, 2018

Conversation

smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Apr 25, 2018

Key changes

  • Remove openshift/origin and openshift/node (no longer supported)
  • Move hypershift and hyperkube and oc to their own images
  • Make control-plane depend on cli
  • Break a lot of bad interdependencies between oc and the server.
  • Reduce the size of the oc binary from 220M to 110M.
  • Deprecate oc export in favor of oc get --export (remove the server dependency)
  • Prepare to completely remove openshift start node by creating openshift-node-config which takes node-config.yaml and writes out kubelet flags (a future PR will switch ansible to use this)
  • Consolidate image builds to use the docker-build image (same content in both) which will prepare to later split the build binary out.
  • Remove a diagnostic that checks master and node config and remove the experimental config validation commands - they pull in almost all of the other binaries. Instead, in the future hyperkube and hypershift should have a --check flag.
  • Prepare to split out openshift-sdn into its own binary by cutting dependencies with the master.

@deads2k getting very close...

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 25, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 26, 2018
@smarterclayton
Copy link
Contributor Author

/retest

@jpeeler
Copy link
Contributor

jpeeler commented Apr 26, 2018

I think openshift/origin-cli needs to be added to hack/build-local-images.py for the catalog job.

@smarterclayton
Copy link
Contributor Author

/test gcp

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Apr 27, 2018

Exact description of image changes in this PR, targeted for 3.10 @jupierce @adammhaile @smunilla

  • New images
    • openshift/origin-hyperkube
    • openshift/origin-hypershift
    • openshift/origin-tests
    • openshift/origin-cli
  • Removed images
    • openshift/node
    • openshift/origin
    • openshift/origin-sti-builder
  • Image parent changes
    • openshift/origin-cli is now parent to openshift/origin-control-plane
    • openshift/origin-base is now parent to openshift/origin-ipfailover-keepalived
  • New RPMs
    • origin-hyperkube (binary removed from origin)
    • origin-hypershift (binary removed from origin)
  • Changed RPMs
    • added openshift-node-config to origin-node

Ansible is already updated to avoid the removed images.

@adammhaile
Copy link
Contributor

@smarterclayton Thanks for the heads up. If possible can we hold off on a merge until I can further update my PR to reflect these changes? As previously discussed I already PR'd the change to remove the STI image but this goes much beyond that. So I would like to have our end entirely ready as this will surely break our 3.10 build.

@adammhaile
Copy link
Contributor

@smarterclayton @jupierce - Looking into these changes further and I have some questions and housekeeping before I can start making changes to our automation:

  • None of the new images have distgits. Can you give me more details on those so that we can request those? Ideally we need a new trello card based of this: https://trello.com/c/1aQdAGZN/398-buildauto-template-for-adding-new-rpms-and-images-to-cd-build-management
  • openshift-enterprise-keepalived-ipfailover was already based on ose-base in our automation. I see it used to be control-plane for origin. I just want to make sure I'm not missing something here as it's no change as far as I can tell from our end.
  • You've said that origin-cli is new and parent to origin-control-plane, but we do not have a control-plane image. Should we also add that one to the OCP 3.10 builds?

@smarterclayton
Copy link
Contributor Author

None of the new images have distgits.

These are in origin, whatever you do for origin sub packages.

@smarterclayton
Copy link
Contributor Author

I just want to make sure I'm not missing something here as it's no change as far as I can tell from our end.

There is no impact, the reparenting is just a detail.

You've said that origin-cli is new and parent to origin-control-plane, but we do not have a control-plane image. Should we also add that one to the OCP 3.10 builds?

Right now openshift3/ose is aliased to openshift3/ose-control-plane. So you could leave that as is for now, but ose-control-plane == ose

Breaks dependencies on the entire controller manager and apiserver stack
in oc. Config diagnostics were of questionable value, validation was
deprecated and experimental.
Move it closer to where we need to test it (config package doesn't bring
those dependencies into scope anymore).
Move the dependency into the master startup flow.
Instead of calling the apiserver code, clear status. Mark deprecated in
favor of oc get --export (which itself is now of questionable value). A
future release should provide better logic for stripping out unnecessary
fields.
Do not remove openshift binary from the node image quite yet.
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 2, 2018
@smarterclayton
Copy link
Contributor Author

/test launch-gcp

Will restore in the future. Also reduce logging level on tests below the
unintelligible threshold.
@smarterclayton smarterclayton added the lgtm Indicates that a PR is ready to be merged. label May 2, 2018
@smarterclayton
Copy link
Contributor Author

Fixed the last service catalog issue, @jpeeler i grabbed two fixes from Jay's PR (but not all of them). This is blocking other changes.

@smarterclayton smarterclayton added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label May 2, 2018
@jpeeler
Copy link
Contributor

jpeeler commented May 2, 2018

@smarterclayton I see you grabbed the change for the configmap resource lock, but not the RBAC for that resource. Am I missing something? FWIW, #19568 is in the merge pool now.

@smarterclayton
Copy link
Contributor Author

/retest

@smarterclayton
Copy link
Contributor Author

/refresh

@smarterclayton
Copy link
Contributor Author

/skip

@smarterclayton
Copy link
Contributor Author

Green buttoning since we need to make the split happen.

@smarterclayton smarterclayton merged commit 10a5f34 into openshift:master May 2, 2018
wozniakjan pushed a commit to wozniakjan/origin that referenced this pull request May 9, 2018
As a result of openshift#19509, packages
`iptables`, `ethtool`, and `socat` ended up being defined twice for
`yum install`. This removes duplicate packages in the same command
adammhaile added a commit to openshift-eng/ocp-build-data that referenced this pull request Oct 31, 2018
adammhaile pushed a commit to openshift-eng/ocp-build-data that referenced this pull request Oct 31, 2018
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. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants