)]}'
{"openvswitch/Dockerfile-DPDK.ubuntu_bionic":[{"author":{"_account_id":16353,"name":"Georg Kunz","email":"georg.kunz@ericsson.com","username":"georg-kunz"},"change_message_id":"d95cc1577490096c4867a748a027cf61d063efc0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"FROM docker.io/ubuntu:bionic"},{"line_number":2,"context_line":"LABEL maintainer\u003d\"cheng1.li@intel.com\""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"9fb8cfa7_3df3b9a0","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":28},"updated":"2019-06-22 20:07:49.000000000","message":"how about renaming this file to \topenvswitch/Dockerfile.ubuntu_bionic-dpdk to keep it in line with the previous Dockerfile.debian-dpdk file?","commit_id":"ce1dec59adf889a8bd86d9f40bf0c2a4a8def274"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"03dda17bba47e6cc6d6096ecb44ad68f20f49f5b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"FROM docker.io/ubuntu:bionic"},{"line_number":2,"context_line":"LABEL maintainer\u003d\"cheng1.li@intel.com\""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"9fb8cfa7_91716ee1","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":28},"in_reply_to":"9fb8cfa7_3df3b9a0","updated":"2019-06-24 03:30:15.000000000","message":"Done","commit_id":"ce1dec59adf889a8bd86d9f40bf0c2a4a8def274"}],"openvswitch/Dockerfile.ubuntu_bionic":[{"author":{"_account_id":16353,"name":"Georg Kunz","email":"georg.kunz@ericsson.com","username":"georg-kunz"},"change_message_id":"e0824585af5ee716a2da4cbeb12e69e06eda94c8","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_5392a358","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"updated":"2019-06-14 13:55:11.000000000","message":"This package installs a dpdk-enabled binary besides the non-dpdk-enabled binary. The non-dpdk-enabled binary remains the default however. As a resolution, we should set the -dpdk-binary as default using the \"update-alternatives\" tool.","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"38b35b8b1984ad1807f43251f26c45d58f8b11c1","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_a06135d5","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_137aebb8","updated":"2019-06-16 09:04:59.000000000","message":"Agree on keeping only one ovs image which has both dpdk and none-dpdk ovs-vswitchd files. And I","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"21ee24d0e44ed4e52042f15b8bb3448f94dc47d5","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_807bd189","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_137aebb8","updated":"2019-06-16 08:51:59.000000000","message":"Agree on using a single one ovs image for both dpdk and none-dpdk.\n\nIn ovs chart, we need to find the right ovs-vswitchd before exec command. It could be logical like below(it doesn\u0027t broke Suse):\n\n```\nOVS_VSWITCHD\u003d/usr/sbin/ovs-vswitchd\nif [ -f /usr/lib/openvswitch-switch-dpdk/ovs-vswitchd-dpdk ]\nthen\n  OVS_VSWITCHD\u003d\u0027/usr/lib/openvswitch-switch-dpdk/ovs-vswitchd-dpdk\u0027\nfi\n```","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"b99a2f4b9a8bbc4b867fcf7dc7b581e7b1cb3f25","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_fcc02d7f","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_2197823d","updated":"2019-06-17 10:00:45.000000000","message":"I didn\u0027t test with SUSE image. I said it doesn\u0027t break SUSE because OVS_SWITCHD fallbacks to the default \u0027/usr/sbin/ovs-vswitchd\u0027 which must exist on any ovs image. And of course these lines should be surrounded by dpdk.enabled condition. Let me paste the whole logical(looking up ovs-vswitchd):\n\n```\nOVS_SWITCHD\u003d/usr/sbin/ovs-vswitchd\nif dpdk.enabled\nthen\n  # for ubuntu bionic\n  if [ -f /usr/lib/openvswitch-switch-dpdk/ovs-vswitchd-dpdk ]\n  then\n    OVS_SWITCHD\u003d/usr/lib/openvswitch-switch-dpdk/ovs-vswitchd-dpdk\n  fi\n  # for suse(if ovs-vswitchd patch is the same with ubuntu bionic, we can save this part)\n  if [ -f /path/to/suse/ovs-vswitchd-dpdk ]\n  then\n    OVS_SWITCHD\u003d/path/to/suse/ovs-vswitchd-dpdk\n  fi\n  # for other distribution OS\nfi\n# We don\u0027t raise error even if no ovs-vswitchd-dpdk binary exists. Because users may customize the ovs image, the default \u0027/usr/sbin/ovs-vswitchd\u0027 could already support dpdk in their ovs image.\n```","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":16353,"name":"Georg Kunz","email":"georg.kunz@ericsson.com","username":"georg-kunz"},"change_message_id":"53c0f48c5e5540c16344178d9ba29122b4568731","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_137aebb8","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_5392a358","updated":"2019-06-14 14:13:42.000000000","message":"Actually, we could think about using this tool during runtime (not in the Dockerfile as I originally thought) and instead call it only of DPDK is enabled in the helm chart. This way, there would be no need for a second dpdk image.\n\nOn the other hand, this would make the helm chart image / OS dependent as these operations are only needed and available on Ubuntu, not Suse for instance.\n\nThoughts?","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":16353,"name":"Georg Kunz","email":"georg.kunz@ericsson.com","username":"georg-kunz"},"change_message_id":"74391cbf1d7f4c02d08738ba62362ab0d103b581","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_910c5395","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_5bd91af2","updated":"2019-06-19 05:01:12.000000000","message":"You are right that there shouldn\u0027t be any host OS-specific if/else statements in the chart\u0027s code. For this reason, Rihab and I attempted to implement the ideas outlined by Jean-Philippe in a multi-distro spec [1]. In a nutshell, the idea is to make all host OS specific parameters part configurable in the chart\u0027s values file in a new \"software\" section. In case of OVS, this would be the path to the OVS binary. The sane default would be /usr/sbin/ovs-vswitchd, but you can override it for DPDK for instance. See the updated OVS chart [2].\n\nThere is currently a problem though we only realized after making and pushing these changes: when using the dpdk-enabled OVS binary in /usr/lib/openvswitch-switch-dpdk/ovs-vswitchd-dpdk, the pod starts up nicely, everything looks fine until it crashes after about 30s and crashloops:\n\ntraps: revalidator82[3578] general protection ip:7f2907df58f0 sp:7f28e67fb8a0 error:0 in libc-2.27.so[7f2907db5000+1e7000]\n\nWe need to investigate this further. If we don\u0027t get this under control, we need to go for 2 separte images.\n\n[1] https://github.com/openstack/openstack-helm/commit/9292a53640468c61f560633968147db6896687f6\n[2] https://review.opendev.org/#/c/626894/28/openvswitch/values.yaml","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":16353,"name":"Georg Kunz","email":"georg.kunz@ericsson.com","username":"georg-kunz"},"change_message_id":"6aca17eacdfd39616dda13c5b32bfd050a319603","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_2197823d","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_a06135d5","updated":"2019-06-17 09:17:06.000000000","message":"Thanks for the pointer to the OVS_SWITCHD varible. Did you also test it with a SUSE image - because you mentioned that this didn\u0027t break it. If so, we should update the helm chart accordingly.\n\nMoreover, for the implementation, I\u0027d like to include a go template conditional which only enables the if-statement above if the dpdk:enabled flag is set in the values.yaml.","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"d9867c6a7408fe7196c9f9323054159fe61f0a1b","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":1,"id":"9fb8cfa7_5bd91af2","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_fcc02d7f","updated":"2019-06-19 01:54:38.000000000","message":"Georg, after thinking more on this. I am a bit prefer to built two ovs images instead of using one single ovs for both dpdk and none-dpdk cases. So that we don\u0027t have to add extra logic code in ovs chart. It\u0027s easier to maintain ovs chart if all ovs-vswitchd binary path is the same in every ovs image. Otherwise, we may need more \u0027if\u0027 conditions for future SUSE, CentOS support. Therefor the ovs chart would be complicated to cover the difference in ovs images. So I would prefer to build individual images for dpdk and none-dpdk cases. The ovs-vswitchd binary path is the same in each image. Actually all other version ovs images is supposed to follow this rule as well. This could be a requirement for openstack-helm ovs image. Your thought?","commit_id":"fe052e06c6146eeb0bc8ca2572623c996487aa9c"},{"author":{"_account_id":17068,"name":"Jean-Philippe Evrard","email":"openstack@a.spamming.party","username":"evrardjp"},"change_message_id":"101fe39c5cbbf46f165aa90d0ecc4af638f110f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9fb8cfa7_f61a776d","updated":"2019-06-18 08:53:47.000000000","message":"This implies that dpdk is used for all the bionic images, which I think we shouldn\u0027t do. Let\u0027s create a dpdk file for this.","commit_id":"ff5d296ac919c060d03a964e7f32191ac7026686"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"952f67d89642d4abcb7820741087550fba4209d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9fb8cfa7_a3203e9d","in_reply_to":"9fb8cfa7_d6e093cf","updated":"2019-06-18 12:36:43.000000000","message":"I will add the comment in next patch.","commit_id":"ff5d296ac919c060d03a964e7f32191ac7026686"},{"author":{"_account_id":16353,"name":"Georg Kunz","email":"georg.kunz@ericsson.com","username":"georg-kunz"},"change_message_id":"dbe6efa31ff329290bb31834a7fc7c72e622fdfc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9fb8cfa7_d6e093cf","in_reply_to":"9fb8cfa7_f61a776d","updated":"2019-06-18 09:24:31.000000000","message":"This installs the dpdk-enabled OVS next to the non-dpdk OVS (which is a package dependency). You could than, in principle, choose at runtime which version to use (the non-dpdk one is the default). This way, we\u0027d only need one OVS image (less stuff to maintain).\n\nRihab is currently prototyping a variant of this which attempts to follow your multi-distro spec, i.e., by exposing config values to define the OVS binary to run.","commit_id":"ff5d296ac919c060d03a964e7f32191ac7026686"},{"author":{"_account_id":17068,"name":"Jean-Philippe Evrard","email":"openstack@a.spamming.party","username":"evrardjp"},"change_message_id":"101fe39c5cbbf46f165aa90d0ecc4af638f110f6","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"9fb8cfa7_960c5b3c","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"updated":"2019-06-18 08:53:47.000000000","message":"that\u0027s super clean. I am surprised you don\u0027t need more packages :)","commit_id":"ff5d296ac919c060d03a964e7f32191ac7026686"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"952f67d89642d4abcb7820741087550fba4209d0","unresolved":false,"context_lines":[{"line_number":3,"context_line":""},{"line_number":4,"context_line":"RUN set -ex;\\"},{"line_number":5,"context_line":"    apt-get update;\\"},{"line_number":6,"context_line":"    apt-get install -y openvswitch-switch-dpdk;\\"},{"line_number":7,"context_line":"    rm -rf /var/lib/apt/lists/*"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"9fb8cfa7_a72feba5","line":6,"range":{"start_line":6,"start_character":23,"end_line":6,"end_character":46},"in_reply_to":"9fb8cfa7_960c5b3c","updated":"2019-06-18 12:36:43.000000000","message":"Most needed packages are installed as dependencies. As we don\u0027t need to compile ovs like debian version so no packages like gcc, make, etc.","commit_id":"ff5d296ac919c060d03a964e7f32191ac7026686"},{"author":{"_account_id":8863,"name":"Andrii Ostapenko","email":"anost1986@gmail.com","username":"aostapenko"},"change_message_id":"cd336d88556b0beba9aa5c6555422c1bb3a7bc5f","unresolved":false,"context_lines":[{"line_number":1,"context_line":"FROM docker.io/ubuntu:bionic"},{"line_number":2,"context_line":"LABEL maintainer\u003d\"cheng1.li@intel.com\""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# openvswitch-switch is also installed as a dependency, it means"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9fb8cfa7_2531a9ed","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":28},"updated":"2019-06-18 21:25:33.000000000","message":"I would prefer this to be\n\nARG FROM\u003ddocker.io/ubuntu:bionic\nFROM ${FROM}\n\nSo custom image with altered apt configuration could be used to install ovs related packages from different source.","commit_id":"40c4fd58b8f1eb9e1b66c8fd6dc7ea1dfc59cf22"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"d9867c6a7408fe7196c9f9323054159fe61f0a1b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"FROM docker.io/ubuntu:bionic"},{"line_number":2,"context_line":"LABEL maintainer\u003d\"cheng1.li@intel.com\""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# openvswitch-switch is also installed as a dependency, it means"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9fb8cfa7_9be552df","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":28},"in_reply_to":"9fb8cfa7_2531a9ed","updated":"2019-06-19 01:54:38.000000000","message":"To my knowledge, custom image is the image which built at custom local env. They need to have their own Dockerfile. This Dockefile is not designed to support individual customers.","commit_id":"40c4fd58b8f1eb9e1b66c8fd6dc7ea1dfc59cf22"},{"author":{"_account_id":29668,"name":"cheng li","email":"cheng1.li@intel.com","username":"chengli3"},"change_message_id":"b817a540cc00192d18a2076928f12115546030d9","unresolved":false,"context_lines":[{"line_number":1,"context_line":"FROM docker.io/ubuntu:bionic"},{"line_number":2,"context_line":"LABEL maintainer\u003d\"cheng1.li@intel.com\""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# openvswitch-switch is also installed as a dependency, it means"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9fb8cfa7_b6494d8c","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":28},"in_reply_to":"9fb8cfa7_7b72dee5","updated":"2019-06-19 03:42:20.000000000","message":"This Dockerfile is for ubuntu bionic only, for now.\nFor ubuntu xenial, we build from source so we can\u0027t reuse the Dockerfile.\n\nSo I would like to add the ARG when we have a real case of reusing the Dockerfile.","commit_id":"40c4fd58b8f1eb9e1b66c8fd6dc7ea1dfc59cf22"},{"author":{"_account_id":8863,"name":"Andrii Ostapenko","email":"anost1986@gmail.com","username":"aostapenko"},"change_message_id":"534510a5cc7ceef55813450c82034b1a18cff5af","unresolved":false,"context_lines":[{"line_number":1,"context_line":"FROM docker.io/ubuntu:bionic"},{"line_number":2,"context_line":"LABEL maintainer\u003d\"cheng1.li@intel.com\""},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"# openvswitch-switch is also installed as a dependency, it means"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9fb8cfa7_7b72dee5","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":28},"in_reply_to":"9fb8cfa7_9be552df","updated":"2019-06-19 02:04:28.000000000","message":"Still, you\u0027re removing any ability to control ovs version, limiting it to what is available in bionic repos. As long as there\u0027s a will to get rid of building from sources, what I suggested does not affect functionality in any way, but allows to reuse this Dockerfile without drawbacks.","commit_id":"40c4fd58b8f1eb9e1b66c8fd6dc7ea1dfc59cf22"}],"zuul.d/openvswitch.yaml":[{"author":{"_account_id":17068,"name":"Jean-Philippe Evrard","email":"openstack@a.spamming.party","username":"evrardjp"},"change_message_id":"101fe39c5cbbf46f165aa90d0ecc4af638f110f6","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            - latest-debian"},{"line_number":46,"context_line":"        - context: openvswitch"},{"line_number":47,"context_line":"          repository: openstackhelm/openvswitch"},{"line_number":48,"context_line":"          dockerfile: Dockerfile.ubuntu_bionic"},{"line_number":49,"context_line":"          tags:"},{"line_number":50,"context_line":"            - latest-ubuntu_bionic"},{"line_number":51,"context_line":"    files: \u0026openvswitch_files"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"9fb8cfa7_7620c79c","line":48,"range":{"start_line":48,"start_character":33,"end_line":48,"end_character":46},"updated":"2019-06-18 08:53:47.000000000","message":"see also my opinion on having a different file, with -dpdk.","commit_id":"ff5d296ac919c060d03a964e7f32191ac7026686"}]}
