Skip to content

Create a kube-controller-manager image that contains ceph-common#22082

Closed
smarterclayton wants to merge 1 commit into
openshift:masterfrom
smarterclayton:status
Closed

Create a kube-controller-manager image that contains ceph-common#22082
smarterclayton wants to merge 1 commit into
openshift:masterfrom
smarterclayton:status

Conversation

@smarterclayton

@smarterclayton smarterclayton commented Feb 19, 2019

Copy link
Copy Markdown
Contributor

Only kube-controller-manager needs access to these binaries. Makes the image 50M larger, probably bigger when we add the other dependencies.

Only kube-controller-manager needs access to these binaries.
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 19, 2019
@smarterclayton

Copy link
Copy Markdown
Contributor Author

/retest

@smarterclayton

Copy link
Copy Markdown
Contributor Author

@mfojtik still assessing the impact here. Talking with @gnufied and reaching out to the ceph team.

For attach/detach, apparently iscsi and others are needed in the controller manager image. @gnufied do you have the list?

@smarterclayton

Copy link
Copy Markdown
Contributor Author

/retest

@gnufied

gnufied commented Feb 20, 2019

Copy link
Copy Markdown
Member

@smarterclayton @mfojtik looks like ceph-common is the only package we need in controller-manager image. iscsi tools aren't needed in controller-manager.

The other thing is - flexvolume binaries in case users want to use flexvolume plugins that support attach/detach but we don't necessarily need to bake anything in the controller-manager image for that.

@smarterclayton

Copy link
Copy Markdown
Contributor Author

/retest

@jsafrane

Copy link
Copy Markdown
Contributor

/retest
/lgtm

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

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@eparis

eparis commented Apr 10, 2019

Copy link
Copy Markdown
Member

/hold
questions about ceph client tools in RHEL8 might change if we do this at all

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 10, 2019
@jsafrane

Copy link
Copy Markdown
Contributor

questions about ceph client tools in RHEL8 might change if we do this at all

See https://bugzilla.redhat.com/show_bug.cgi?id=1665912 for details

@eparis

eparis commented Jun 27, 2019

Copy link
Copy Markdown
Member

/close
at this point we only expect to support ceph via CSI, not via flex or the built in driver. As such we do not plan to merge this PR.

@openshift-ci-robot

Copy link
Copy Markdown

@eparis: Closed this PR.

Details

In response to this:

/close
at this point we only expect to support ceph via CSI, not via flex or the built in driver. As such we do not plan to merge this PR.

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants