Skip to content

nexedi / slapos.core

Go to a project
Merge request !46 opened by  

grid: new shared_part_list config file option

This new option for slapos.cfg allow to define a list of paths to use for shared parts feature of slapos.recipe.cmmi. This will be injected in the buildout profile used to install software, as ${buildout:shared-part-list} which is the option slapos.recipe.cmmi use since 0.11.

Note that the option changed in slapos.recipe.cmmi because this was incompatible. In 0.10, the option was named ${buildout:shared-parts} and it was supposed to be a single path. slapos.recipe.cmmi 0.10 was trying to create this path. If we continued using this ${buildout:shared-parts} option and changed to passe a newline-separated list of paths, then slapos.recipe.cmmi would just create one directory with \n in the name and will probably fail, which would make all software from 1.0.73 to 1.0.121 not installable on next slapos.core, so we used another buildout option to prevent this problem.

Because generally, profiles using slapos.recipe.cmmi 0.10 were not "safely" shared, we don't even provide compatibility with old option.

{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Jérome Perrin @jerome

    Marked this merge request as a Work In Progress

    Marked this merge request as a Work In Progress

    Toggle commit list
  • Jérome Perrin @jerome

    mentioned in merge request slapos.recipe.cmmi!6 (merged)

    mentioned in merge request slapos.recipe.cmmi!6 (merged)

    Toggle commit list
  • Jérome Perrin @jerome commented
    Owner

    @rafael @xiaowu.zhang this MR is a placeholder for slapos node software part of slapos.recipe.cmmi!6 (merged) , as suggested in slapos.recipe.cmmi!6 (comment 60883)

  • Jérome Perrin @jerome commented
    Owner

    I'm wondering if we should define one path or multiple paths to support the use case of webrunner (or testnode) using the cache of "parent" slapos.

    In that case, slapos from webrunner or testnode would not be able to write to this cache (because no permission to write in that folder), but can still use softwares from this shared path.

    One problem is that it would be harder to track which softwares are using the shared when we want to remove obsolete entries from the shared folder.

    Also slapos.recipe.cmmi would be a bit more complex.

  • Rafael Monnerat @rafael commented
    Owner

    you can also include download-cache option too

  • Rafael Monnerat @rafael commented
    Owner

    In fact, I think here:

    https://lab.nexedi.com/nexedi/slapos.core/blob/master/slapos/grid/SlapObject.py#L275

    should generate something like:

    [buildout]
extends = %(software_release)s

%(additional_options)s

    I don't know why we append options on the command line, instead just write a buildout.cfg that actually works out of the box.

  • Jérome Perrin @jerome commented
    Owner

    @rafael thanks for comments. I just realize you suggested in slapos.recipe.cmmi!6 (comment 60867) to name this option shared-part and here I used slapos-recipe-cmmi-shared-path name. I have no strong preference for either name. I started using slapos-recipe-cmmi-shared-path before noticing your suggestion.

    That's a good point that we could generate a "ready to use" buildout in the first place. I just copied existing behaviour of passing options on command line, but it seems better to add this in generated buildout.

  • Rafael Monnerat @rafael

    Added 1 commit:

    • b9f401ba - [shared]: Rename slapos_recipe_cmmi_shared_path into shared_software_root

    Added 1 commit:

    • b9f401ba - [shared]: Rename slapos_recipe_cmmi_shared_path into shared_software_root
    Toggle commit list
  • Rafael Monnerat @rafael

    Added 1 commit:

    • 9d9b3680 - wip: Create shared_software_root folders

    Added 1 commit:

    • 9d9b3680 - wip: Create shared_software_root folders
    Toggle commit list
  • Jérome Perrin
    @jerome started a discussion on an old version of the diff
    Last updated by Rafael Monnerat
    slapos/format.py
    540 541 network_interface_name = self.interface.name
    541 542 self._addUniqueLocalAddressIpv6(network_interface_name)
    542 543  
    543 for path in self.instance_root, self.software_root:
    544 path_list = [self.instance_root, self.software_root]
    545
    546 if self.shared_software_root is not None:
    547 path_list.extend(self.shared_software_root.split(",")]
    • Jérome Perrin @jerome commented
      Owner

      @rafael I don't think we should use ',' as path separator. We could use os.pathsep but for consistency with other options in slapos.cfg ( for example download-from-binary-cache-url-blacklist ), why not be\n separated, one path per line ?

      shared_software_root =
  /opt/slapgrid/shared/
  /srv/slapgrid/slappart0/srv/runner/software/shared/
    • Rafael Monnerat @rafael commented
      Owner

      sure, pathsep is better

    Please register or sign in to reply
  • Xiaowu Zhang @xiaowu.zhang commented
    Developer

    how can i test this patch ? it seems i can't test in my webrunner. i changed slapos.core, but the one i changed is not used

  • Rafael Monnerat @rafael commented
    Owner

    Like everthing else related to egg "python setup.py test"

  • Rafael Monnerat @rafael commented
    Owner

    this patch is unfinished probably, as tests may fail (I may not finish myself, it is up to you to finish)

  • Xiaowu Zhang @xiaowu.zhang

    Added 1 commit:

    • 65b4bfba - use \n for consistency with other options

    Added 1 commit:

    • 65b4bfba - use \n for consistency with other options
    Toggle commit list
  • Jérome Perrin @jerome commented
    Owner

    @rafael and I discussed this one year ago in Lille. We said that it's important to support the "recursivity" of slapos, so that when we have a "slapos in slapos", the inner slapos can reuse shared parts from the outer slapos, but cannot write to shared parts of outer slapos.

    If I remember well what we said with

    • in slapos.cfg we should define a list of shared paths, the idea is that the last (or first I forgot) one is read-write, others are read-only.
    • slapos.recipe.cmmi supports this and try to find existing shared parts in all shared path and if not found, it will use the read-write one.
    • code creating a recursive slapos ( testnode, webrunner, standalone ...) needs to pass their own list of shared path as read-only when creating a slapos.
  • Jérome Perrin @jerome

    mentioned in merge request slapos!624 (merged)

    mentioned in merge request slapos!624 (merged)

    Toggle commit list
  • Jérome Perrin @jerome

    mentioned in merge request slapos.recipe.cmmi!9 (merged)

    mentioned in merge request slapos.recipe.cmmi!9 (merged)

    Toggle commit list
  • Jérome Perrin @jerome

    added 713 commits

    Compare with previous version

    added 713 commits

    Compare with previous version

    Toggle commit list
  • Jérome Perrin @jerome commented
    Owner

    @luke @rafael I updated this part as well. I'm still testing, but I already appreciate feedback. I changed to add in the generated buildout.cfg as you suggested.

  • Rafael Monnerat @rafael commented
    Owner

    @jerome looks fine the latest changes

  • Jérome Perrin @jerome

    added 1 commit

    • dbab6068 - grid: new shared_part_list config file option

    Compare with previous version

    added 1 commit

    • dbab6068 - grid: new shared_part_list config file option

    Compare with previous version

    Toggle commit list
  • Jérome Perrin @jerome

    added 41 commits

    Compare with previous version

    added 41 commits

    Compare with previous version

    Toggle commit list
  • Jérome Perrin @jerome

    unmarked as a Work In Progress

    unmarked as a Work In Progress

    Toggle commit list
  • Jérome Perrin @jerome

    changed title from WIP: Support for slapos.recipe.cmmi in slapos node software to grid: new shared_part_list config file option

    changed title from WIP: Support for slapos.recipe.cmmi in slapos node software to grid: new shared_part_list config file option

    Toggle commit list
  • Jérome Perrin @jerome commented
    Owner

    I update to use another buildout option, because we cannot make the same option become a list without breaking old software releases that assume that it's a single value.

    tests: on py2 ✔ on py3 ✔

  • Jérome Perrin @jerome

    mentioned in merge request !64 (merged)

    mentioned in merge request !64 (merged)

    Toggle commit list
  • Jérome Perrin @jerome commented
    Owner

    If this and slapos.recipe.cmmi!9 (merged) looks OK, it would be nice to see how we can merge and release eggs (if we can make a release of slapos.core with this and !64 (merged) that would be perfect :))

  • Jérome Perrin @jerome

    mentioned in merge request slapos!629 (merged)

    mentioned in merge request slapos!629 (merged)

    Toggle commit list
  • Jérome Perrin @jerome

    mentioned in commit 5cbd70bc

    mentioned in commit 5cbd70bc

    Toggle commit list
  • Jérome Perrin @jerome

    merged

    merged

    Toggle commit list
  • Łukasz Nowak @luke commented
    Master

    I am afraid that shared_part_list usage is incompatible with binary cache upload, as SlapObject.uploadSoftwareRelease tarballs only the SR, and does nothing regarding shared parts. Am I right? I see no test coverage for this case.

    I propose to make shared_part_list mutually exclusive with any option which would result with uploading binary cache. IMO the best way is to fail hard with error if upload could happen with shared_part_list enabled, thus we will protect shacache against bad SRs to be uploaded.

  • Rafael Monnerat @rafael commented
    Owner

    Yes it is incompatible, we won't use on production shared_part_list for now.

  • Jérome Perrin @jerome commented
    Owner

    I am afraid that shared_part_list usage is incompatible with binary cache upload, as SlapObject.uploadSoftwareRelease tarballs only the SR, and does nothing regarding shared parts.

    Something was done in the first batch of changes to support shared software. I think that 19e3f1b6 is already doing what you suggest to do. But I don't know what is supposed to create this .shared. I have some software installed using shared parts and there's no .shared.

    If we want to support this, maybe it's easier/more reliable to introspect the content of .installed.cfg to see if the some shared parts are referenced.

    I see no test coverage for this case.

    Yes, it seems there is no test for that .shared thing, but there is no test for uploading either (I only see tests checking that blacklisted software don't upload) . Maybe we should uploading software to shacache ?

  • Jérome Perrin @jerome commented
    Owner

    Maybe we should uploading software to shacache ?

    typo ... Maybe we should abandon uploading software to shacache ?

  • Łukasz Nowak @luke commented
    Master

    If we want to support this, maybe it's easier/more reliable to introspect the content of .installed.cfg to see if the some shared parts are referenced.

    I'll give it a try.

  • Łukasz Nowak @luke commented
    Master

    typo ... Maybe we should abandon uploading software to shacache ?

    For Vifib team uploading to binary cache (which happens during slapos node software run on some servers automatically) is a critical feature to have good deployments.

  • Łukasz Nowak @luke commented
    Master

    I'll give it a try.

    Proposed in !152

  • Thomas Gambier
    @tomo started a discussion on the diff
    Resolved by Thomas Gambier
    slapos.cfg.example
    8 8 certificate_repository_path = /etc/opt/slapos/ssl/partition_pki
    9 9 software_root = /opt/slapgrid
    10 10 instance_root = /srv/slapgrid
    11 # A list of directories containing shared parts for slapos.recipe.cmmi.
    12 # Only the last one is used to install new parts, other ones can have
    13 # already installed parts.
    14 shared_part_list =
    • Thomas Gambier @tomo commented
      Master

      wow, I didn't see this change when you commited. Since slapos node register is actually using slapos.cfg.example to create slapos.cfg, it means all our newly installed machine will have this option on by default.

      Either we let it here this way and we change our playbook to comment those 3 lines. Or by default we comment those 3 lines.

    • Łukasz Nowak @luke commented
      Master

      Either we let it here this way and we change our playbook to comment those 3 lines. Or by default we comment those 3 lines.

      I think it shall be only disabled on servers which upload to binary cache, no?

    • Thomas Gambier @tomo commented
      Master

      I think it shall be only disabled on servers which upload to binary cache, no?

      No, because I want all our servers to be interchangeable : we should be able to move one webrunner from one place to another place without the user noticing the difference. If some of our servers have shared parts and some others don't, you may end up with a webrunner relying on the top level shared parts that won't work after it moves to another computer.

      For now, I consider shared parts feature not suitable for slapos at system level.

    • Łukasz Nowak @luke commented
      Master

      If some of our servers have shared parts and some others don't, you may end up with a webrunner relying on the top level shared parts that won't work after it moves to another computer.

      Ok, sure! I missed that important part.

    • Jérome Perrin @jerome commented
      Owner

      wow I did not realised it was used in slapos node register. Nothing create these folders, so this probably means that nodes configured with slapos node register cannot install software.

      Maybe we should amend this slapos.cfg.example so that the lines are only here as comment ?

    • Thomas Gambier @tomo commented
      Master

      I just did in c0710b65

      I will schedule a new release of slapos-node package soon so that we don't have this in newly installled machine.

    Please register or sign in to reply
  • Thomas Gambier @tomo

    resolved all discussions

    resolved all discussions

    Toggle commit list
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备14008524号