Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-6792

Stop making repo changes to target different ZFS in Intel build farm

Details

    • Task
    • Resolution: Unresolved
    • Minor
    • None
    • None
    • None
    • 9223372036854775807

    Description

      It should not be necessary to make changes to Lustre's source tree just to start building Lustre against a different minor version of ZFS. Lustre on the master branch can be built successfully against a number of recent ZFS releases. It should not be necessary to make a Lustre source tree change to, for instance, start using ZFS 0.6.4.2 in Intel's build farm instead of ZFS 0.6.4.1. That is a build farm decision, not a source code implied limitation.

      Build farm decisions should be separate from the Lustre source tree.

      Attachments

        Issue Links

          Activity

            [LU-6792] Stop making repo changes to target different ZFS in Intel build farm
            mdiep Minh Diep added a comment -

            I agree with Chris that hardcode zfs version in lbuild is bad; and yes, we build a zfs version just to test one version at a given time, not the exclusive version. Perhaps the compatible version should go into README or release-note so user can see without looking into the codes.

            I think we can implement the zfs version similar to kernel version in target files. ie 2.6-rhel6.6.target.in. lbuild sources this file before building zfs to it should pick up the version.

            mdiep Minh Diep added a comment - I agree with Chris that hardcode zfs version in lbuild is bad; and yes, we build a zfs version just to test one version at a given time, not the exclusive version. Perhaps the compatible version should go into README or release-note so user can see without looking into the codes. I think we can implement the zfs version similar to kernel version in target files. ie 2.6-rhel6.6.target.in. lbuild sources this file before building zfs to it should pick up the version.

            This enables tracking an exact shippable. So that builds in the past can be reproduced without guess as to what version of ZFS/SPL was used.

            There are many fine ways to track your exact shippable without embedding a fixed version number in Lustre's source tree. Please change to one that doesn't require a Lustre tree code change.

            This documents what the latest version of ZFS is that has been tested.

            Since lbuild is part of Intel's build farm, not part of Lustre's build system, hiding that detail in lbuild is pretty sneaky. That would seem to violate the Principal of Least Astonishment. Seriously, you can delete the lbuild directory and Lustre still builds just fine (perhaps only needed a minor tweak to adjust for the loss of that directory). lbuild is not the place to communicate important details testing. Not to mention that this value is set before any testing is performed, so at some point it time it only represent what version will be tested in the future...and then it says nothing about what amount of testing has happened. It only really tells us that the Intel's automated testing from that point on would have target the new version.

            And furthermore, no one but Intel can even change that value, because a coordinating change in the Intel build farm is needed. Since a change to Intel's build farm is needed to pull in the new ZFS packages, why not just make the current build target part of that build farm as well? That would seem to be the least astonishing place to put that setting to, I imagine, most people that have never seen lbuild.

            Build environment changes should not magically break source-code (Principal of Least Astonishment). This forces a developer to make any required ZFS specific changes (autoconf, and feature specific tests) when upgrading the compatible ZFS version.

            It sounds like you were agreeing with me with that last point. I would just clarify again that Lustre is often compatible with multiple versions of ZFS, so the version listed in lbuild is not the compatible ZFS version, it is merely a compatible ZFS version.

            morrone Christopher Morrone (Inactive) added a comment - This enables tracking an exact shippable. So that builds in the past can be reproduced without guess as to what version of ZFS/SPL was used. There are many fine ways to track your exact shippable without embedding a fixed version number in Lustre's source tree. Please change to one that doesn't require a Lustre tree code change. This documents what the latest version of ZFS is that has been tested. Since lbuild is part of Intel's build farm, not part of Lustre's build system, hiding that detail in lbuild is pretty sneaky. That would seem to violate the Principal of Least Astonishment. Seriously, you can delete the lbuild directory and Lustre still builds just fine (perhaps only needed a minor tweak to adjust for the loss of that directory). lbuild is not the place to communicate important details testing. Not to mention that this value is set before any testing is performed, so at some point it time it only represent what version will be tested in the future...and then it says nothing about what amount of testing has happened. It only really tells us that the Intel's automated testing from that point on would have target the new version. And furthermore, no one but Intel can even change that value, because a coordinating change in the Intel build farm is needed. Since a change to Intel's build farm is needed to pull in the new ZFS packages, why not just make the current build target part of that build farm as well? That would seem to be the least astonishing place to put that setting to, I imagine, most people that have never seen lbuild. Build environment changes should not magically break source-code (Principal of Least Astonishment). This forces a developer to make any required ZFS specific changes (autoconf, and feature specific tests) when upgrading the compatible ZFS version. It sounds like you were agreeing with me with that last point. I would just clarify again that Lustre is often compatible with multiple versions of ZFS, so the version listed in lbuild is not the compatible ZFS version, it is merely a compatible ZFS version.
            utopiabound Nathaniel Clark added a comment - - edited

            My thoughts on hard coded ZFS/SPL version numbers:

            • This enables tracking an exact shippable. So that builds in the past can be reproduced without guess as to what version of ZFS/SPL was used.
            • This documents what the latest version of ZFS is that has been tested.
            • Build environment changes should not magically break source-code (Principal of Least Astonishment). This forces a developer to make any required ZFS specific changes (autoconf, and feature specific tests) when upgrading the compatible ZFS version.
            utopiabound Nathaniel Clark added a comment - - edited My thoughts on hard coded ZFS/SPL version numbers: This enables tracking an exact shippable. So that builds in the past can be reproduced without guess as to what version of ZFS/SPL was used. This documents what the latest version of ZFS is that has been tested. Build environment changes should not magically break source-code ( Principal of Least Astonishment ). This forces a developer to make any required ZFS specific changes (autoconf, and feature specific tests) when upgrading the compatible ZFS version.

            If there is legitimately a compatibility requirement, tests for those ZFS differences need to be expressed in autoconf tests and in the packaging system build requirements (e.g. in the spec file for RPM based systems). lbuild is not the place to express any kind build requirements.

            lbuild is not part of Lustre's build system, it is part of Intel's build farm. Many people do not use lbuild at all when building and packaging Lustre. For that reason alone, lbuild is not the correct place to hide any expression of compatibility.

            Also, while ZFS APIs changed somewhat rapidly in the past, those changes have settled down. Lustre is not tied to a single specific version of ZFS, and pretending that it is in the lbuild script has no bearing on reality.

            The hard coding of the ZFS version in lbuild really is just a function of your build farm. It is not an inherent Lustre build issue.

            morrone Christopher Morrone (Inactive) added a comment - If there is legitimately a compatibility requirement, tests for those ZFS differences need to be expressed in autoconf tests and in the packaging system build requirements (e.g. in the spec file for RPM based systems). lbuild is not the place to express any kind build requirements. lbuild is not part of Lustre's build system, it is part of Intel's build farm. Many people do not use lbuild at all when building and packaging Lustre. For that reason alone, lbuild is not the correct place to hide any expression of compatibility. Also, while ZFS APIs changed somewhat rapidly in the past, those changes have settled down. Lustre is not tied to a single specific version of ZFS, and pretending that it is in the lbuild script has no bearing on reality. The hard coding of the ZFS version in lbuild really is just a function of your build farm. It is not an inherent Lustre build issue.

            Chris, this version serves as a record of which ZFS version a particular version of Lustre is known to build against and run with. With every ZFS release there are minor changes needed for building or correct functionality. For older versions of Lustre it generally isn't possible to run with the latest/arbitrary version of ZFS due to those changes, so e.g. 2.5.3 needs to build with 0.6.3 and won't work with 0.6.4. The same is true with the ability of Lustre to build against arbitrary versions of the Linux kernel - there needs to be a record in the tree that indicates which kernels (client and server) it is known to work with.

            adilger Andreas Dilger added a comment - Chris, this version serves as a record of which ZFS version a particular version of Lustre is known to build against and run with. With every ZFS release there are minor changes needed for building or correct functionality. For older versions of Lustre it generally isn't possible to run with the latest/arbitrary version of ZFS due to those changes, so e.g. 2.5.3 needs to build with 0.6.3 and won't work with 0.6.4. The same is true with the ability of Lustre to build against arbitrary versions of the Linux kernel - there needs to be a record in the tree that indicates which kernels (client and server) it is known to work with.

            Essentially, it makes little sense to hard code a ZFS version into Lustre's source tree. There is not a representation of some inherent Lustre code dependency, it is merely a build farm target choice.

            This is an Intel build farm configuration decision, and should be made somewhere other than the main Lustre source tree. A build farm configuration file would be a logical place for such a thing.

            morrone Christopher Morrone (Inactive) added a comment - Essentially, it makes little sense to hard code a ZFS version into Lustre's source tree. There is not a representation of some inherent Lustre code dependency, it is merely a build farm target choice. This is an Intel build farm configuration decision, and should be made somewhere other than the main Lustre source tree. A build farm configuration file would be a logical place for such a thing.
            pjones Peter Jones added a comment -

            Hi Chris

            I'm not sure that I understand your concerns here. Let's chat about this on our next call

            Peter

            pjones Peter Jones added a comment - Hi Chris I'm not sure that I understand your concerns here. Let's chat about this on our next call Peter

            People

              pjones Peter Jones
              morrone Christopher Morrone (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated: