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

DAOS-7229 build: Remove client dependence on server libraries #5445

Merged
merged 9 commits into from
Apr 16, 2021

Conversation

jolivier23
Copy link
Contributor

@jolivier23 jolivier23 commented Apr 14, 2021

Client package currently installs PMDK and SPDK which we do not
want. Changes in this patch:

  1. Move storage_estimator to VOS
  2. Replace libdaos_common_pmem in daos_test with libdaos_common
  3. Move io_conf (VOS only) to server package
  4. Move libvos_size.so to daos_srv
  5. Remove daos_common_pemem dependence from daos utility
  6. Remove rdbt from common package
  7. Remove libdaos_tests and various utilities from common package
  8. Modify daos.spec to move files around

Signed-off-by: Jeff Olivier jeffrey.v.olivier@intel.com

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5445/1/execution/node/328/log

@daosbuild1
Copy link
Collaborator

Test stage Unit Test completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-5445/1/display/redirect

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5445/2/execution/node/352/log

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5445/3/execution/node/365/log

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5445/4/execution/node/365/log

Client package currently installs PMDK and SPDK which we do not
want.  Changes in this patch:

1. Move storage_estimator to VOS
2. Replace libdaos_common_pmem in daos_test with libdaos_common
3. Move io_conf (VOS only) to server package
4. Move libvos_size.so to daos_srv
5. Remove daos_common_pemem dependence from daos utility
6. Remove rdbt from common package
7. Remove libdaos_tests and various utilities from common package
8. Modify daos.spec to move files around

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@jolivier23 jolivier23 changed the title DAOs-7229 build: Remove client dependence on server libraries DAOS-7229 build: Remove client dependence on server libraries Apr 14, 2021
@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5445/5/execution/node/372/log

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

johannlombardi
johannlombardi previously approved these changes Apr 15, 2021
brianjmurrell
brianjmurrell previously approved these changes Apr 15, 2021
Copy link
Contributor

@brianjmurrell brianjmurrell left a comment

Choose a reason for hiding this comment

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

The changes look good, but since this is something that can very easily regress, it would be great to add a regression test at

sudo yum -y install --exclude ompi daos{,-client}-"${DAOS_PKG_VERSION}"
sudo yum -y history rollback last-1
to ensure that the daos-server RPM is not installed with the daos-client RPM.

If you are going to add that, could you also include this change (I've included the regression test(s); I'm assuming that daos-server should not depend on daos-client; you can remove the second regression test if that is not a valid assumption):

-sudo yum -y install --exclude ompi daos{,-client}-"${DAOS_PKG_VERSION}"
-sudo yum -y history rollback last-1
-sudo yum -y install --exclude ompi daos{,-{server,client}}-"${DAOS_PKG_VERSION}"
-sudo yum -y install --exclude ompi daos{,-tests}-"${DAOS_PKG_VERSION}"
+sudo yum -y install --exclude ompi daos-client-"${DAOS_PKG_VERSION}"
+if rpm -q daos-server; then
+    echo "daos-server RPM should not be installed as a dependency of daos-client"
+    exit 1
+fi
+sudo yum -y history rollback last-1
+sudo yum -y install --exclude ompi daos-server-"${DAOS_PKG_VERSION}"
+if rpm -q daos-server; then
+    echo "daos-client RPM should not be installed as a dependency of daos-server"
+    exit 1
+fi
+sudo yum -y install --exclude ompi daos-tests-"${DAOS_PKG_VERSION}"

We also should not be papering over possible daos-{server,client} and daos dependency issues by forcing the install of the latter with the former.

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
@jolivier23 jolivier23 dismissed stale reviews from brianjmurrell and johannlombardi via 9756f27 April 15, 2021 15:42
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Test CentOS 7 RPMs completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5445/8/execution/node/1085/log

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

johannlombardi
johannlombardi previously approved these changes Apr 15, 2021
Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
@daosbuild1
Copy link
Collaborator

Test stage NLT completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-5445/10/display/redirect

Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

Copy link
Contributor

@brianjmurrell brianjmurrell left a comment

Choose a reason for hiding this comment

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

Great work on this @jolivier23!

@jolivier23 jolivier23 merged commit 3d5a44b into master Apr 16, 2021
@jolivier23 jolivier23 deleted the jvolivie/storage branch April 16, 2021 15:55
jolivier23 added a commit that referenced this pull request Apr 16, 2021
Client package currently installs PMDK and SPDK which we do not
want.  Changes in this patch:

1. Move storage_estimator to VOS
2. Replace libdaos_common_pmem in daos_test with libdaos_common
3. Move io_conf (VOS only) to server package
4. Move libvos_size.so to daos_srv
5. Remove daos_common_pemem dependence from daos utility
6. Remove rdbt from common package
7. Remove libdaos_tests and various utilities from common package
8. Modify daos.spec to move files around

Note that daos-tests now depends on daos-server.  We will need to do some further changes to remove this dependence.  But at least there is no dependence between daos-server and daos-client

Signed-off-by: Jeff Olivier <jeffrey.v.olivier@intel.com>
@ashleypittman ashleypittman mentioned this pull request Apr 28, 2021
@ashleypittman ashleypittman mentioned this pull request May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants