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

install ceph-common in control plane so rbd provisioner can find rbd cli to create rbd images #19870

Merged

Conversation

rootfs
Copy link
Member

@rootfs rootfs commented May 29, 2018

@openshift-ci-robot openshift-ci-robot requested a review from dcbw May 29, 2018 17:39
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 29, 2018
@sdodson
Copy link
Member

sdodson commented May 29, 2018

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 29, 2018
@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

smarterclayton commented May 29, 2018 via email

@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 May 29, 2018
@@ -16,14 +16,12 @@ COPY system-container/manifest.json system-container/config.json.template system
RUN INSTALL_PKGS=" \
origin-hyperkube origin-node origin-sdn-ovs \
socat ethtool device-mapper iptables nmap-ncat e2fsprogs \
xfsprogs device-mapper-persistent-data ceph-common \
xfsprogs device-mapper-persistent-data\
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove this here?

Copy link
Member

Choose a reason for hiding this comment

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

Because node is FROM openshift/origin-control-plane

Copy link
Contributor

Choose a reason for hiding this comment

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

It has to remain here. Node image won’t depend on control plane next release

@@ -6,7 +6,9 @@
#
FROM openshift/origin-cli

RUN INSTALL_PKGS="origin" && \
RUN INSTALL_PKGS="origin ceph-common" && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this only used by the controllers?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's used by controller-manager for dynamic provisioning and resize and by kubelet to map/unmap (local "attach"/"detach") volumes.

@rootfs
Copy link
Member Author

rootfs commented Jun 15, 2018

@smarterclayton @childsb yes we need ceph-common to pick up rbd command for controller to provision and resize rbd images.

@@ -16,14 +16,12 @@ COPY system-container/manifest.json system-container/config.json.template system
RUN INSTALL_PKGS=" \
origin-hyperkube origin-node origin-sdn-ovs \
socat ethtool device-mapper iptables nmap-ncat e2fsprogs \
xfsprogs device-mapper-persistent-data ceph-common \
xfsprogs device-mapper-persistent-data\
Copy link
Contributor

Choose a reason for hiding this comment

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

It has to remain here. Node image won’t depend on control plane next release

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 21, 2018
@rootfs
Copy link
Member Author

rootfs commented Jun 21, 2018

@sdodson @smarterclayton thanks, review addressed.

cc @bertinatto

libmnl libnetfilter_conntrack conntrack-tools \
libnfnetlink iproute bridge-utils procps-ng openssl \
binutils xz kmod-libs kmod sysvinit-tools device-mapper-libs dbus \
iscsi-initiator-utils \
" && \
yum install -y centos-release-ceph-luminous && \
rpm -V centos-release-ceph-luminous && \
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to still install this here if it isn’t already. You should remove all changes to this file.

@rootfs rootfs force-pushed the rbd-img-control-plane branch from 73e84ce to 77a396c Compare June 21, 2018 15:23
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 21, 2018
@rootfs rootfs force-pushed the rbd-img-control-plane branch from 77a396c to 4d2a0a1 Compare June 21, 2018 15:25
@rootfs
Copy link
Member Author

rootfs commented Jun 21, 2018

@smarterclayton thanks, updated, PTAL

@smarterclayton
Copy link
Contributor

/lgtm
/approve
/cherry-pick release-3.10

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2018
@jsafrane
Copy link
Contributor

@rootfs, can you please squash the commits?

@rootfs rootfs force-pushed the rbd-img-control-plane branch from 4d2a0a1 to f65605a Compare June 21, 2018 17:00
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2018
@rootfs
Copy link
Member Author

rootfs commented Jun 21, 2018

@jsafrane squashed

@jsafrane
Copy link
Contributor

/lgtm
/hold cancel

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 21, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, rootfs, sdodson, 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

@bertinatto
Copy link
Member

/retest

1 similar comment
@smarterclayton
Copy link
Contributor

/retest

@openshift-merge-robot openshift-merge-robot merged commit 67a36ff into openshift:master Jun 21, 2018
@sdodson
Copy link
Member

sdodson commented Jun 22, 2018

/cherrypick release-3.10

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/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.

8 participants