diff mbox series

[ovs-dev,v3,6/8] utilities/docker: Fix up container build.

Message ID 20240321231535.695233-7-dceara@redhat.com
State Accepted
Headers show
Series Various treewide fixes. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Dumitru Ceara March 21, 2024, 11:15 p.m. UTC
Most of the steps were inaccurate.  Instead, use latest Ubuntu, use
OVS from the submodule inside the OVN repo.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 utilities/docker/Makefile          |  4 ++--
 utilities/docker/debian/Dockerfile |  2 +-
 utilities/docker/debian/build.sh   |  2 ++
 utilities/docker/install_ovn.sh    | 31 ++++++++++++++----------------
 4 files changed, 19 insertions(+), 20 deletions(-)

Comments

Mark Michelson March 22, 2024, 6:34 p.m. UTC | #1
On 3/21/24 19:15, Dumitru Ceara wrote:
> Most of the steps were inaccurate.  Instead, use latest Ubuntu, use
> OVS from the submodule inside the OVN repo.
> 
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> ---
>   utilities/docker/Makefile          |  4 ++--
>   utilities/docker/debian/Dockerfile |  2 +-
>   utilities/docker/debian/build.sh   |  2 ++
>   utilities/docker/install_ovn.sh    | 31 ++++++++++++++----------------
>   4 files changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
> index 57e95651cb..aad9c3482c 100644
> --- a/utilities/docker/Makefile
> +++ b/utilities/docker/Makefile
> @@ -1,5 +1,5 @@
> -#export OVN_BRANCH=master
> -#export OVN_VERSION=2.12
> +#export OVN_BRANCH=main
> +#export OVN_VERSION=24.03.90

Is this something that we should update with each major release of OVN? 
If so, I could probably alter my release script to include updating 
utilities/docker/Makefile as part of the release patches.

>   #export DISTRO=debian
>   #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
>   #export DOCKER_REPO=ovn-org/ovn
> diff --git a/utilities/docker/debian/Dockerfile b/utilities/docker/debian/Dockerfile
> index 366ad6d4f3..a89ef46c9f 100644
> --- a/utilities/docker/debian/Dockerfile
> +++ b/utilities/docker/debian/Dockerfile
> @@ -1,4 +1,4 @@
> -FROM ubuntu:16.04
> +FROM ubuntu:22.04
>   MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com>
>   
>   ARG OVN_BRANCH
> diff --git a/utilities/docker/debian/build.sh b/utilities/docker/debian/build.sh
> index 57ace5f505..6edb5b85e4 100755
> --- a/utilities/docker/debian/build.sh
> +++ b/utilities/docker/debian/build.sh
> @@ -12,6 +12,8 @@
>   # See the License for the specific language governing permissions and
>   # limitations under the License.
>   
> +set -e
> +
>   OVN_BRANCH=$1
>   GITHUB_SRC=$2
>   
> diff --git a/utilities/docker/install_ovn.sh b/utilities/docker/install_ovn.sh
> index 55c189aaee..5157da1497 100755
> --- a/utilities/docker/install_ovn.sh
> +++ b/utilities/docker/install_ovn.sh
> @@ -12,29 +12,26 @@
>   # See the License for the specific language governing permissions and
>   # limitations under the License.
>   
> +set -e
> +
>   OVN_BRANCH=$1
>   GITHUB_SRC=$2
>   
> -# get ovs source always from master as its needed as dependency
> -mkdir /build; cd /build
> -git clone --depth 1 -b master https://github.com/openvswitch/ovs.git
> -cd ovs;
> -mkdir _gcc;
> +# Get ovn source.
> +git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
> +cd ovn
>   
> -# build and install
> +# Get OVS submodule, build and install OVS.
> +git submodule update --init
> +cd ovs
>   ./boot.sh
> -cd _gcc
> -../configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
> +./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
>   --enable-ssl
> -cd ..; make -C _gcc install; cd ..
> -
> +make -j8 install
> +cd ..
>   
> -# get ovn source
> -git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
> -cd ovn
> -
> -# build and install
> +# Build and install OVN.
>   ./boot.sh
>   ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
> ---enable-ssl --with-ovs-source=/build/ovs/ --with-ovs-build=/build/ovs/_gcc
> -make -j8; make install
> +--enable-ssl
> +make -j8 install
Dumitru Ceara March 25, 2024, 8:47 a.m. UTC | #2
On 3/22/24 19:34, Mark Michelson wrote:
> On 3/21/24 19:15, Dumitru Ceara wrote:
>> Most of the steps were inaccurate.  Instead, use latest Ubuntu, use
>> OVS from the submodule inside the OVN repo.
>>
>> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
>> ---
>>   utilities/docker/Makefile          |  4 ++--
>>   utilities/docker/debian/Dockerfile |  2 +-
>>   utilities/docker/debian/build.sh   |  2 ++
>>   utilities/docker/install_ovn.sh    | 31 ++++++++++++++----------------
>>   4 files changed, 19 insertions(+), 20 deletions(-)
>>
>> diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
>> index 57e95651cb..aad9c3482c 100644
>> --- a/utilities/docker/Makefile
>> +++ b/utilities/docker/Makefile
>> @@ -1,5 +1,5 @@
>> -#export OVN_BRANCH=master
>> -#export OVN_VERSION=2.12
>> +#export OVN_BRANCH=main
>> +#export OVN_VERSION=24.03.90
> 
> Is this something that we should update with each major release of OVN?
> If so, I could probably alter my release script to include updating
> utilities/docker/Makefile as part of the release patches.
> 

It's just a comment and I guess the original intention was to show users
how to set this up.  But, back to your question, if it's not a lot of
work, automatically changing this on release would be nice.

Also, it might make sense to set up a CI job that actually runs this in
CI, maybe periodically.  I can try to find some time to do that at some
point in the future.  Wdyt?

>>   #export DISTRO=debian
>>   #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
>>   #export DOCKER_REPO=ovn-org/ovn
>> diff --git a/utilities/docker/debian/Dockerfile
>> b/utilities/docker/debian/Dockerfile
>> index 366ad6d4f3..a89ef46c9f 100644
>> --- a/utilities/docker/debian/Dockerfile
>> +++ b/utilities/docker/debian/Dockerfile
>> @@ -1,4 +1,4 @@
>> -FROM ubuntu:16.04
>> +FROM ubuntu:22.04
>>   MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com>
>>     ARG OVN_BRANCH
>> diff --git a/utilities/docker/debian/build.sh
>> b/utilities/docker/debian/build.sh
>> index 57ace5f505..6edb5b85e4 100755
>> --- a/utilities/docker/debian/build.sh
>> +++ b/utilities/docker/debian/build.sh
>> @@ -12,6 +12,8 @@
>>   # See the License for the specific language governing permissions and
>>   # limitations under the License.
>>   +set -e
>> +
>>   OVN_BRANCH=$1
>>   GITHUB_SRC=$2
>>   diff --git a/utilities/docker/install_ovn.sh
>> b/utilities/docker/install_ovn.sh
>> index 55c189aaee..5157da1497 100755
>> --- a/utilities/docker/install_ovn.sh
>> +++ b/utilities/docker/install_ovn.sh
>> @@ -12,29 +12,26 @@
>>   # See the License for the specific language governing permissions and
>>   # limitations under the License.
>>   +set -e
>> +
>>   OVN_BRANCH=$1
>>   GITHUB_SRC=$2
>>   -# get ovs source always from master as its needed as dependency
>> -mkdir /build; cd /build
>> -git clone --depth 1 -b master https://github.com/openvswitch/ovs.git
>> -cd ovs;
>> -mkdir _gcc;
>> +# Get ovn source.
>> +git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
>> +cd ovn
>>   -# build and install
>> +# Get OVS submodule, build and install OVS.
>> +git submodule update --init
>> +cd ovs
>>   ./boot.sh
>> -cd _gcc
>> -../configure --localstatedir="/var" --sysconfdir="/etc"
>> --prefix="/usr" \
>> +./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
>>   --enable-ssl
>> -cd ..; make -C _gcc install; cd ..
>> -
>> +make -j8 install
>> +cd ..
>>   -# get ovn source
>> -git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
>> -cd ovn
>> -
>> -# build and install
>> +# Build and install OVN.
>>   ./boot.sh
>>   ./configure --localstatedir="/var" --sysconfdir="/etc"
>> --prefix="/usr" \
>> ---enable-ssl --with-ovs-source=/build/ovs/
>> --with-ovs-build=/build/ovs/_gcc
>> -make -j8; make install
>> +--enable-ssl
>> +make -j8 install
>
Ales Musil March 25, 2024, 9:09 a.m. UTC | #3
On Mon, Mar 25, 2024 at 9:47 AM Dumitru Ceara <dceara@redhat.com> wrote:

> On 3/22/24 19:34, Mark Michelson wrote:
> > On 3/21/24 19:15, Dumitru Ceara wrote:
> >> Most of the steps were inaccurate.  Instead, use latest Ubuntu, use
> >> OVS from the submodule inside the OVN repo.
> >>
> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> >> ---
> >>   utilities/docker/Makefile          |  4 ++--
> >>   utilities/docker/debian/Dockerfile |  2 +-
> >>   utilities/docker/debian/build.sh   |  2 ++
> >>   utilities/docker/install_ovn.sh    | 31 ++++++++++++++----------------
> >>   4 files changed, 19 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
> >> index 57e95651cb..aad9c3482c 100644
> >> --- a/utilities/docker/Makefile
> >> +++ b/utilities/docker/Makefile
> >> @@ -1,5 +1,5 @@
> >> -#export OVN_BRANCH=master
> >> -#export OVN_VERSION=2.12
> >> +#export OVN_BRANCH=main
> >> +#export OVN_VERSION=24.03.90
> >
> > Is this something that we should update with each major release of OVN?
> > If so, I could probably alter my release script to include updating
> > utilities/docker/Makefile as part of the release patches.
> >
>
> It's just a comment and I guess the original intention was to show users
> how to set this up.  But, back to your question, if it's not a lot of
> work, automatically changing this on release would be nice.
>
> Also, it might make sense to set up a CI job that actually runs this in
> CI, maybe periodically.  I can try to find some time to do that at some
> point in the future.  Wdyt?
>


Hi Mark, Dumitru,

from a different perspective, I would be interested to see if this is still
used by anyone. We have other methods of running OVN in containers that are
up to date and maintained. It might be the case of working perfectly, still
used and doesn't need any attention. But it would probably still be useful
to hear from the community if that's the case. IMO we shouldn't run CI on
something just for the sake of running CI.

Does that make sense?


>
> >>   #export DISTRO=debian
> >>   #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
> >>   #export DOCKER_REPO=ovn-org/ovn
> >> diff --git a/utilities/docker/debian/Dockerfile
> >> b/utilities/docker/debian/Dockerfile
> >> index 366ad6d4f3..a89ef46c9f 100644
> >> --- a/utilities/docker/debian/Dockerfile
> >> +++ b/utilities/docker/debian/Dockerfile
> >> @@ -1,4 +1,4 @@
> >> -FROM ubuntu:16.04
> >> +FROM ubuntu:22.04
> >>   MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com>
> >>     ARG OVN_BRANCH
> >> diff --git a/utilities/docker/debian/build.sh
> >> b/utilities/docker/debian/build.sh
> >> index 57ace5f505..6edb5b85e4 100755
> >> --- a/utilities/docker/debian/build.sh
> >> +++ b/utilities/docker/debian/build.sh
> >> @@ -12,6 +12,8 @@
> >>   # See the License for the specific language governing permissions and
> >>   # limitations under the License.
> >>   +set -e
> >> +
> >>   OVN_BRANCH=$1
> >>   GITHUB_SRC=$2
> >>   diff --git a/utilities/docker/install_ovn.sh
> >> b/utilities/docker/install_ovn.sh
> >> index 55c189aaee..5157da1497 100755
> >> --- a/utilities/docker/install_ovn.sh
> >> +++ b/utilities/docker/install_ovn.sh
> >> @@ -12,29 +12,26 @@
> >>   # See the License for the specific language governing permissions and
> >>   # limitations under the License.
> >>   +set -e
> >> +
> >>   OVN_BRANCH=$1
> >>   GITHUB_SRC=$2
> >>   -# get ovs source always from master as its needed as dependency
> >> -mkdir /build; cd /build
> >> -git clone --depth 1 -b master https://github.com/openvswitch/ovs.git
> >> -cd ovs;
> >> -mkdir _gcc;
> >> +# Get ovn source.
> >> +git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
> >> +cd ovn
> >>   -# build and install
> >> +# Get OVS submodule, build and install OVS.
> >> +git submodule update --init
> >> +cd ovs
> >>   ./boot.sh
> >> -cd _gcc
> >> -../configure --localstatedir="/var" --sysconfdir="/etc"
> >> --prefix="/usr" \
> >> +./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr"
> \
> >>   --enable-ssl
> >> -cd ..; make -C _gcc install; cd ..
> >> -
> >> +make -j8 install
> >> +cd ..
> >>   -# get ovn source
> >> -git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
> >> -cd ovn
> >> -
> >> -# build and install
> >> +# Build and install OVN.
> >>   ./boot.sh
> >>   ./configure --localstatedir="/var" --sysconfdir="/etc"
> >> --prefix="/usr" \
> >> ---enable-ssl --with-ovs-source=/build/ovs/
> >> --with-ovs-build=/build/ovs/_gcc
> >> -make -j8; make install
> >> +--enable-ssl
> >> +make -j8 install
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Thanks,
Ales
Mark Michelson March 26, 2024, 6:08 p.m. UTC | #4
On 3/25/24 05:09, Ales Musil wrote:
> 
> 
> On Mon, Mar 25, 2024 at 9:47 AM Dumitru Ceara <dceara@redhat.com 
> <mailto:dceara@redhat.com>> wrote:
> 
>     On 3/22/24 19:34, Mark Michelson wrote:
>      > On 3/21/24 19:15, Dumitru Ceara wrote:
>      >> Most of the steps were inaccurate.  Instead, use latest Ubuntu, use
>      >> OVS from the submodule inside the OVN repo.
>      >>
>      >> Signed-off-by: Dumitru Ceara <dceara@redhat.com
>     <mailto:dceara@redhat.com>>
>      >> ---
>      >>   utilities/docker/Makefile          |  4 ++--
>      >>   utilities/docker/debian/Dockerfile |  2 +-
>      >>   utilities/docker/debian/build.sh   |  2 ++
>      >>   utilities/docker/install_ovn.sh    | 31
>     ++++++++++++++----------------
>      >>   4 files changed, 19 insertions(+), 20 deletions(-)
>      >>
>      >> diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
>      >> index 57e95651cb..aad9c3482c 100644
>      >> --- a/utilities/docker/Makefile
>      >> +++ b/utilities/docker/Makefile
>      >> @@ -1,5 +1,5 @@
>      >> -#export OVN_BRANCH=master
>      >> -#export OVN_VERSION=2.12
>      >> +#export OVN_BRANCH=main
>      >> +#export OVN_VERSION=24.03.90
>      >
>      > Is this something that we should update with each major release
>     of OVN?
>      > If so, I could probably alter my release script to include updating
>      > utilities/docker/Makefile as part of the release patches.
>      >
> 
>     It's just a comment and I guess the original intention was to show users
>     how to set this up.  But, back to your question, if it's not a lot of
>     work, automatically changing this on release would be nice.
> 
>     Also, it might make sense to set up a CI job that actually runs this in
>     CI, maybe periodically.  I can try to find some time to do that at some
>     point in the future.  Wdyt?
> 
> 
> 
> Hi Mark, Dumitru,
> 
> from a different perspective, I would be interested to see if this is 
> still used by anyone. We have other methods of running OVN in containers 
> that are up to date and maintained. It might be the case of working 
> perfectly, still used and doesn't need any attention. But it would 
> probably still be useful to hear from the community if that's the case. 
> IMO we shouldn't run CI on something just for the sake of running CI.
> 
> Does that make sense?

First off, with regards to this patch, I think it can be merged as-is, 
with no further action. If we want to continue this discussion, we 
shouldn't let it hold up the patch.

When it comes to your questions, Ales, they're really hard to answer :)

Is anyone using utilities/docker? I have no idea. I'd be willing to bet 
most CMSes are using their own containerization methods instead of the 
in-tree method. However, I don't know if there are people that use it 
for development purposes. I know I tend to use ovn-fake-multinode during 
development/testing, but that's just me.

When you mention having other methods of running OVN in containers, are 
you referring to the content in utilities/containers ? I agree that when 
it comes to CI, we probably don't need the content in utilities/docker . 
But it appears to have been written to be a general-purpose method of 
creating versioned container builds of OVN, rather than specifically for 
CI. The content in utilities/docker is also quite out of date, I would 
imagine, since it hasn't been updated since adding cluster support in 
January 2020. There is also documentation throughout the tree that 
refers to the content in utilities/docker. If a new user were to 
download the OVN source and browse the documentation, they would try to 
use the Makefile in utilities/docker.

In my opinion, the best thing to do is to merge the two containerization 
methods together into a single unified method. We can then use this for 
our CI, meaning we will keep it up to date and add necessary features as 
we see fit. This way we aren't keeping a "dead" method alive in the tree 
for no reason, and we won't be adding unnecessary CI either.

What do you think?

> 
> 
>      >>   #export DISTRO=debian
>      >>   #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
>     <https://github.com/ovn-org/ovn.git>
>      >>   #export DOCKER_REPO=ovn-org/ovn
>      >> diff --git a/utilities/docker/debian/Dockerfile
>      >> b/utilities/docker/debian/Dockerfile
>      >> index 366ad6d4f3..a89ef46c9f 100644
>      >> --- a/utilities/docker/debian/Dockerfile
>      >> +++ b/utilities/docker/debian/Dockerfile
>      >> @@ -1,4 +1,4 @@
>      >> -FROM ubuntu:16.04
>      >> +FROM ubuntu:22.04
>      >>   MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com
>     <mailto:aginwala@ebay.com>>
>      >>     ARG OVN_BRANCH
>      >> diff --git a/utilities/docker/debian/build.sh
>      >> b/utilities/docker/debian/build.sh
>      >> index 57ace5f505..6edb5b85e4 100755
>      >> --- a/utilities/docker/debian/build.sh
>      >> +++ b/utilities/docker/debian/build.sh
>      >> @@ -12,6 +12,8 @@
>      >>   # See the License for the specific language governing
>     permissions and
>      >>   # limitations under the License.
>      >>   +set -e
>      >> +
>      >>   OVN_BRANCH=$1
>      >>   GITHUB_SRC=$2
>      >>   diff --git a/utilities/docker/install_ovn.sh
>      >> b/utilities/docker/install_ovn.sh
>      >> index 55c189aaee..5157da1497 100755
>      >> --- a/utilities/docker/install_ovn.sh
>      >> +++ b/utilities/docker/install_ovn.sh
>      >> @@ -12,29 +12,26 @@
>      >>   # See the License for the specific language governing
>     permissions and
>      >>   # limitations under the License.
>      >>   +set -e
>      >> +
>      >>   OVN_BRANCH=$1
>      >>   GITHUB_SRC=$2
>      >>   -# get ovs source always from master as its needed as dependency
>      >> -mkdir /build; cd /build
>      >> -git clone --depth 1 -b master
>     https://github.com/openvswitch/ovs.git
>     <https://github.com/openvswitch/ovs.git>
>      >> -cd ovs;
>      >> -mkdir _gcc;
>      >> +# Get ovn source.
>      >> +git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
>      >> +cd ovn
>      >>   -# build and install
>      >> +# Get OVS submodule, build and install OVS.
>      >> +git submodule update --init
>      >> +cd ovs
>      >>   ./boot.sh
>      >> -cd _gcc
>      >> -../configure --localstatedir="/var" --sysconfdir="/etc"
>      >> --prefix="/usr" \
>      >> +./configure --localstatedir="/var" --sysconfdir="/etc"
>     --prefix="/usr" \
>      >>   --enable-ssl
>      >> -cd ..; make -C _gcc install; cd ..
>      >> -
>      >> +make -j8 install
>      >> +cd ..
>      >>   -# get ovn source
>      >> -git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
>      >> -cd ovn
>      >> -
>      >> -# build and install
>      >> +# Build and install OVN.
>      >>   ./boot.sh
>      >>   ./configure --localstatedir="/var" --sysconfdir="/etc"
>      >> --prefix="/usr" \
>      >> ---enable-ssl --with-ovs-source=/build/ovs/
>      >> --with-ovs-build=/build/ovs/_gcc
>      >> -make -j8; make install
>      >> +--enable-ssl
>      >> +make -j8 install
>      >
> 
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org <mailto:dev@openvswitch.org>
>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>     <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
> 
> 
> Thanks,
> Ales
> -- 
> 
> Ales Musil
> 
> Senior Software Engineer - OVN Core
> 
> Red Hat EMEA <https://www.redhat.com>
> 
> amusil@redhat.com <mailto:amusil@redhat.com>
> 
> <https://red.ht/sig>
>
Ales Musil March 27, 2024, 6:40 a.m. UTC | #5
On Tue, Mar 26, 2024 at 7:09 PM Mark Michelson <mmichels@redhat.com> wrote:

> On 3/25/24 05:09, Ales Musil wrote:
> >
> >
> > On Mon, Mar 25, 2024 at 9:47 AM Dumitru Ceara <dceara@redhat.com
> > <mailto:dceara@redhat.com>> wrote:
> >
> >     On 3/22/24 19:34, Mark Michelson wrote:
> >      > On 3/21/24 19:15, Dumitru Ceara wrote:
> >      >> Most of the steps were inaccurate.  Instead, use latest Ubuntu,
> use
> >      >> OVS from the submodule inside the OVN repo.
> >      >>
> >      >> Signed-off-by: Dumitru Ceara <dceara@redhat.com
> >     <mailto:dceara@redhat.com>>
> >      >> ---
> >      >>   utilities/docker/Makefile          |  4 ++--
> >      >>   utilities/docker/debian/Dockerfile |  2 +-
> >      >>   utilities/docker/debian/build.sh   |  2 ++
> >      >>   utilities/docker/install_ovn.sh    | 31
> >     ++++++++++++++----------------
> >      >>   4 files changed, 19 insertions(+), 20 deletions(-)
> >      >>
> >      >> diff --git a/utilities/docker/Makefile
> b/utilities/docker/Makefile
> >      >> index 57e95651cb..aad9c3482c 100644
> >      >> --- a/utilities/docker/Makefile
> >      >> +++ b/utilities/docker/Makefile
> >      >> @@ -1,5 +1,5 @@
> >      >> -#export OVN_BRANCH=master
> >      >> -#export OVN_VERSION=2.12
> >      >> +#export OVN_BRANCH=main
> >      >> +#export OVN_VERSION=24.03.90
> >      >
> >      > Is this something that we should update with each major release
> >     of OVN?
> >      > If so, I could probably alter my release script to include
> updating
> >      > utilities/docker/Makefile as part of the release patches.
> >      >
> >
> >     It's just a comment and I guess the original intention was to show
> users
> >     how to set this up.  But, back to your question, if it's not a lot of
> >     work, automatically changing this on release would be nice.
> >
> >     Also, it might make sense to set up a CI job that actually runs this
> in
> >     CI, maybe periodically.  I can try to find some time to do that at
> some
> >     point in the future.  Wdyt?
> >
> >
> >
> > Hi Mark, Dumitru,
> >
> > from a different perspective, I would be interested to see if this is
> > still used by anyone. We have other methods of running OVN in containers
> > that are up to date and maintained. It might be the case of working
> > perfectly, still used and doesn't need any attention. But it would
> > probably still be useful to hear from the community if that's the case.
> > IMO we shouldn't run CI on something just for the sake of running CI.
> >
> > Does that make sense?
>
> First off, with regards to this patch, I think it can be merged as-is,
> with no further action. If we want to continue this discussion, we
> shouldn't let it hold up the patch.
>

 Yeah definitely, I don't want to block this at all, I was just curious.


> When it comes to your questions, Ales, they're really hard to answer :)
>
> Is anyone using utilities/docker? I have no idea. I'd be willing to bet
> most CMSes are using their own containerization methods instead of the
> in-tree method. However, I don't know if there are people that use it
> for development purposes. I know I tend to use ovn-fake-multinode during
> development/testing, but that's just me.
>
> When you mention having other methods of running OVN in containers, are
> you referring to the content in utilities/containers ?


I was mainly referring to ovn-fake-multinode in that case.



> I agree that when
> it comes to CI, we probably don't need the content in utilities/docker .
> But it appears to have been written to be a general-purpose method of
> creating versioned container builds of OVN, rather than specifically for
> CI. The content in utilities/docker is also quite out of date, I would
> imagine, since it hasn't been updated since adding cluster support in
> January 2020. There is also documentation throughout the tree that
> refers to the content in utilities/docker. If a new user were to
> download the OVN source and browse the documentation, they would try to
> use the Makefile in utilities/docker.
>

> In my opinion, the best thing to do is to merge the two containerization
> methods together into a single unified method. We can then use this for
> our CI, meaning we will keep it up to date and add necessary features as
> we see fit. This way we aren't keeping a "dead" method alive in the tree
> for no reason, and we won't be adding unnecessary CI either.
>
> What do you think?
>

I'm not sure merging is the right thing, as there is probably not one size
to fit them all. We could certainly change the documentation with reference
to the utilities/containers as a way to have reproducible builds.

Anyway let's proceed with the series as is and we can always follow up with
other ideas for this area.


>
> >
> >
> >      >>   #export DISTRO=debian
> >      >>   #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
> >     <https://github.com/ovn-org/ovn.git>
> >      >>   #export DOCKER_REPO=ovn-org/ovn
> >      >> diff --git a/utilities/docker/debian/Dockerfile
> >      >> b/utilities/docker/debian/Dockerfile
> >      >> index 366ad6d4f3..a89ef46c9f 100644
> >      >> --- a/utilities/docker/debian/Dockerfile
> >      >> +++ b/utilities/docker/debian/Dockerfile
> >      >> @@ -1,4 +1,4 @@
> >      >> -FROM ubuntu:16.04
> >      >> +FROM ubuntu:22.04
> >      >>   MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com
> >     <mailto:aginwala@ebay.com>>
> >      >>     ARG OVN_BRANCH
> >      >> diff --git a/utilities/docker/debian/build.sh
> >      >> b/utilities/docker/debian/build.sh
> >      >> index 57ace5f505..6edb5b85e4 100755
> >      >> --- a/utilities/docker/debian/build.sh
> >      >> +++ b/utilities/docker/debian/build.sh
> >      >> @@ -12,6 +12,8 @@
> >      >>   # See the License for the specific language governing
> >     permissions and
> >      >>   # limitations under the License.
> >      >>   +set -e
> >      >> +
> >      >>   OVN_BRANCH=$1
> >      >>   GITHUB_SRC=$2
> >      >>   diff --git a/utilities/docker/install_ovn.sh
> >      >> b/utilities/docker/install_ovn.sh
> >      >> index 55c189aaee..5157da1497 100755
> >      >> --- a/utilities/docker/install_ovn.sh
> >      >> +++ b/utilities/docker/install_ovn.sh
> >      >> @@ -12,29 +12,26 @@
> >      >>   # See the License for the specific language governing
> >     permissions and
> >      >>   # limitations under the License.
> >      >>   +set -e
> >      >> +
> >      >>   OVN_BRANCH=$1
> >      >>   GITHUB_SRC=$2
> >      >>   -# get ovs source always from master as its needed as
> dependency
> >      >> -mkdir /build; cd /build
> >      >> -git clone --depth 1 -b master
> >     https://github.com/openvswitch/ovs.git
> >     <https://github.com/openvswitch/ovs.git>
> >      >> -cd ovs;
> >      >> -mkdir _gcc;
> >      >> +# Get ovn source.
> >      >> +git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
> >      >> +cd ovn
> >      >>   -# build and install
> >      >> +# Get OVS submodule, build and install OVS.
> >      >> +git submodule update --init
> >      >> +cd ovs
> >      >>   ./boot.sh
> >      >> -cd _gcc
> >      >> -../configure --localstatedir="/var" --sysconfdir="/etc"
> >      >> --prefix="/usr" \
> >      >> +./configure --localstatedir="/var" --sysconfdir="/etc"
> >     --prefix="/usr" \
> >      >>   --enable-ssl
> >      >> -cd ..; make -C _gcc install; cd ..
> >      >> -
> >      >> +make -j8 install
> >      >> +cd ..
> >      >>   -# get ovn source
> >      >> -git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
> >      >> -cd ovn
> >      >> -
> >      >> -# build and install
> >      >> +# Build and install OVN.
> >      >>   ./boot.sh
> >      >>   ./configure --localstatedir="/var" --sysconfdir="/etc"
> >      >> --prefix="/usr" \
> >      >> ---enable-ssl --with-ovs-source=/build/ovs/
> >      >> --with-ovs-build=/build/ovs/_gcc
> >      >> -make -j8; make install
> >      >> +--enable-ssl
> >      >> +make -j8 install
> >      >
> >
> >     _______________________________________________
> >     dev mailing list
> >     dev@openvswitch.org <mailto:dev@openvswitch.org>
> >     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >     <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
> >
> >
> > Thanks,
> > Ales
> > --
> >
> > Ales Musil
> >
> > Senior Software Engineer - OVN Core
> >
> > Red Hat EMEA <https://www.redhat.com>
> >
> > amusil@redhat.com <mailto:amusil@redhat.com>
> >
> > <https://red.ht/sig>
> >
>
>
Thanks,
Ales
Dumitru Ceara March 28, 2024, 9:53 a.m. UTC | #6
On 3/27/24 07:40, Ales Musil wrote:
> On Tue, Mar 26, 2024 at 7:09 PM Mark Michelson <mmichels@redhat.com> wrote:
> 
>> On 3/25/24 05:09, Ales Musil wrote:
>>>
>>>
>>> On Mon, Mar 25, 2024 at 9:47 AM Dumitru Ceara <dceara@redhat.com
>>> <mailto:dceara@redhat.com>> wrote:
>>>
>>>     On 3/22/24 19:34, Mark Michelson wrote:
>>>      > On 3/21/24 19:15, Dumitru Ceara wrote:
>>>      >> Most of the steps were inaccurate.  Instead, use latest Ubuntu,
>> use
>>>      >> OVS from the submodule inside the OVN repo.
>>>      >>
>>>      >> Signed-off-by: Dumitru Ceara <dceara@redhat.com
>>>     <mailto:dceara@redhat.com>>
>>>      >> ---
>>>      >>   utilities/docker/Makefile          |  4 ++--
>>>      >>   utilities/docker/debian/Dockerfile |  2 +-
>>>      >>   utilities/docker/debian/build.sh   |  2 ++
>>>      >>   utilities/docker/install_ovn.sh    | 31
>>>     ++++++++++++++----------------
>>>      >>   4 files changed, 19 insertions(+), 20 deletions(-)
>>>      >>
>>>      >> diff --git a/utilities/docker/Makefile
>> b/utilities/docker/Makefile
>>>      >> index 57e95651cb..aad9c3482c 100644
>>>      >> --- a/utilities/docker/Makefile
>>>      >> +++ b/utilities/docker/Makefile
>>>      >> @@ -1,5 +1,5 @@
>>>      >> -#export OVN_BRANCH=master
>>>      >> -#export OVN_VERSION=2.12
>>>      >> +#export OVN_BRANCH=main
>>>      >> +#export OVN_VERSION=24.03.90
>>>      >
>>>      > Is this something that we should update with each major release
>>>     of OVN?
>>>      > If so, I could probably alter my release script to include
>> updating
>>>      > utilities/docker/Makefile as part of the release patches.
>>>      >
>>>
>>>     It's just a comment and I guess the original intention was to show
>> users
>>>     how to set this up.  But, back to your question, if it's not a lot of
>>>     work, automatically changing this on release would be nice.
>>>
>>>     Also, it might make sense to set up a CI job that actually runs this
>> in
>>>     CI, maybe periodically.  I can try to find some time to do that at
>> some
>>>     point in the future.  Wdyt?
>>>
>>>
>>>
>>> Hi Mark, Dumitru,
>>>
>>> from a different perspective, I would be interested to see if this is
>>> still used by anyone. We have other methods of running OVN in containers
>>> that are up to date and maintained. It might be the case of working
>>> perfectly, still used and doesn't need any attention. But it would
>>> probably still be useful to hear from the community if that's the case.
>>> IMO we shouldn't run CI on something just for the sake of running CI.
>>>
>>> Does that make sense?
>>
>> First off, with regards to this patch, I think it can be merged as-is,
>> with no further action. If we want to continue this discussion, we
>> shouldn't let it hold up the patch.
>>
> 
>  Yeah definitely, I don't want to block this at all, I was just curious.
> 
> 
>> When it comes to your questions, Ales, they're really hard to answer :)
>>
>> Is anyone using utilities/docker? I have no idea. I'd be willing to bet
>> most CMSes are using their own containerization methods instead of the
>> in-tree method. However, I don't know if there are people that use it
>> for development purposes. I know I tend to use ovn-fake-multinode during
>> development/testing, but that's just me.
>>
>> When you mention having other methods of running OVN in containers, are
>> you referring to the content in utilities/containers ?
> 
> 
> I was mainly referring to ovn-fake-multinode in that case.
> 
> 
> 
>> I agree that when
>> it comes to CI, we probably don't need the content in utilities/docker .
>> But it appears to have been written to be a general-purpose method of
>> creating versioned container builds of OVN, rather than specifically for
>> CI. The content in utilities/docker is also quite out of date, I would
>> imagine, since it hasn't been updated since adding cluster support in
>> January 2020. There is also documentation throughout the tree that
>> refers to the content in utilities/docker. If a new user were to
>> download the OVN source and browse the documentation, they would try to
>> use the Makefile in utilities/docker.
>>
> 
>> In my opinion, the best thing to do is to merge the two containerization
>> methods together into a single unified method. We can then use this for
>> our CI, meaning we will keep it up to date and add necessary features as
>> we see fit. This way we aren't keeping a "dead" method alive in the tree
>> for no reason, and we won't be adding unnecessary CI either.
>>
>> What do you think?
>>
> 
> I'm not sure merging is the right thing, as there is probably not one size
> to fit them all. We could certainly change the documentation with reference
> to the utilities/containers as a way to have reproducible builds.
> 

Another problem with merging the two containerization methods is that
sometimes we need to hack around limitations in our CI.  See the crun
workaround for GH actions for example.

> Anyway let's proceed with the series as is and we can always follow up with
> other ideas for this area.
> 

I'll go ahead with merging this series.  We can improve as a follow up.

Thanks for all the feedback!

> 
>>
>>>
>>>
>>>      >>   #export DISTRO=debian
>>>      >>   #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
>>>     <https://github.com/ovn-org/ovn.git>
>>>      >>   #export DOCKER_REPO=ovn-org/ovn
>>>      >> diff --git a/utilities/docker/debian/Dockerfile
>>>      >> b/utilities/docker/debian/Dockerfile
>>>      >> index 366ad6d4f3..a89ef46c9f 100644
>>>      >> --- a/utilities/docker/debian/Dockerfile
>>>      >> +++ b/utilities/docker/debian/Dockerfile
>>>      >> @@ -1,4 +1,4 @@
>>>      >> -FROM ubuntu:16.04
>>>      >> +FROM ubuntu:22.04
>>>      >>   MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com
>>>     <mailto:aginwala@ebay.com>>
>>>      >>     ARG OVN_BRANCH
>>>      >> diff --git a/utilities/docker/debian/build.sh
>>>      >> b/utilities/docker/debian/build.sh
>>>      >> index 57ace5f505..6edb5b85e4 100755
>>>      >> --- a/utilities/docker/debian/build.sh
>>>      >> +++ b/utilities/docker/debian/build.sh
>>>      >> @@ -12,6 +12,8 @@
>>>      >>   # See the License for the specific language governing
>>>     permissions and
>>>      >>   # limitations under the License.
>>>      >>   +set -e
>>>      >> +
>>>      >>   OVN_BRANCH=$1
>>>      >>   GITHUB_SRC=$2
>>>      >>   diff --git a/utilities/docker/install_ovn.sh
>>>      >> b/utilities/docker/install_ovn.sh
>>>      >> index 55c189aaee..5157da1497 100755
>>>      >> --- a/utilities/docker/install_ovn.sh
>>>      >> +++ b/utilities/docker/install_ovn.sh
>>>      >> @@ -12,29 +12,26 @@
>>>      >>   # See the License for the specific language governing
>>>     permissions and
>>>      >>   # limitations under the License.
>>>      >>   +set -e
>>>      >> +
>>>      >>   OVN_BRANCH=$1
>>>      >>   GITHUB_SRC=$2
>>>      >>   -# get ovs source always from master as its needed as
>> dependency
>>>      >> -mkdir /build; cd /build
>>>      >> -git clone --depth 1 -b master
>>>     https://github.com/openvswitch/ovs.git
>>>     <https://github.com/openvswitch/ovs.git>
>>>      >> -cd ovs;
>>>      >> -mkdir _gcc;
>>>      >> +# Get ovn source.
>>>      >> +git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
>>>      >> +cd ovn
>>>      >>   -# build and install
>>>      >> +# Get OVS submodule, build and install OVS.
>>>      >> +git submodule update --init
>>>      >> +cd ovs
>>>      >>   ./boot.sh
>>>      >> -cd _gcc
>>>      >> -../configure --localstatedir="/var" --sysconfdir="/etc"
>>>      >> --prefix="/usr" \
>>>      >> +./configure --localstatedir="/var" --sysconfdir="/etc"
>>>     --prefix="/usr" \
>>>      >>   --enable-ssl
>>>      >> -cd ..; make -C _gcc install; cd ..
>>>      >> -
>>>      >> +make -j8 install
>>>      >> +cd ..
>>>      >>   -# get ovn source
>>>      >> -git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
>>>      >> -cd ovn
>>>      >> -
>>>      >> -# build and install
>>>      >> +# Build and install OVN.
>>>      >>   ./boot.sh
>>>      >>   ./configure --localstatedir="/var" --sysconfdir="/etc"
>>>      >> --prefix="/usr" \
>>>      >> ---enable-ssl --with-ovs-source=/build/ovs/
>>>      >> --with-ovs-build=/build/ovs/_gcc
>>>      >> -make -j8; make install
>>>      >> +--enable-ssl
>>>      >> +make -j8 install
>>>      >
>>>
>>>     _______________________________________________
>>>     dev mailing list
>>>     dev@openvswitch.org <mailto:dev@openvswitch.org>
>>>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>     <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>>>
>>>
>>> Thanks,
>>> Ales
>>> --
>>>
>>> Ales Musil
>>>
>>> Senior Software Engineer - OVN Core
>>>
>>> Red Hat EMEA <https://www.redhat.com>
>>>
>>> amusil@redhat.com <mailto:amusil@redhat.com>
>>>
>>> <https://red.ht/sig>
>>>
>>
>>
> Thanks,
> Ales
diff mbox series

Patch

diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
index 57e95651cb..aad9c3482c 100644
--- a/utilities/docker/Makefile
+++ b/utilities/docker/Makefile
@@ -1,5 +1,5 @@ 
-#export OVN_BRANCH=master
-#export OVN_VERSION=2.12
+#export OVN_BRANCH=main
+#export OVN_VERSION=24.03.90
 #export DISTRO=debian
 #export GITHUB_SRC=https://github.com/ovn-org/ovn.git
 #export DOCKER_REPO=ovn-org/ovn
diff --git a/utilities/docker/debian/Dockerfile b/utilities/docker/debian/Dockerfile
index 366ad6d4f3..a89ef46c9f 100644
--- a/utilities/docker/debian/Dockerfile
+++ b/utilities/docker/debian/Dockerfile
@@ -1,4 +1,4 @@ 
-FROM ubuntu:16.04
+FROM ubuntu:22.04
 MAINTAINER "Aliasgar Ginwala" <aginwala@ebay.com>
 
 ARG OVN_BRANCH
diff --git a/utilities/docker/debian/build.sh b/utilities/docker/debian/build.sh
index 57ace5f505..6edb5b85e4 100755
--- a/utilities/docker/debian/build.sh
+++ b/utilities/docker/debian/build.sh
@@ -12,6 +12,8 @@ 
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+set -e
+
 OVN_BRANCH=$1
 GITHUB_SRC=$2
 
diff --git a/utilities/docker/install_ovn.sh b/utilities/docker/install_ovn.sh
index 55c189aaee..5157da1497 100755
--- a/utilities/docker/install_ovn.sh
+++ b/utilities/docker/install_ovn.sh
@@ -12,29 +12,26 @@ 
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+set -e
+
 OVN_BRANCH=$1
 GITHUB_SRC=$2
 
-# get ovs source always from master as its needed as dependency
-mkdir /build; cd /build
-git clone --depth 1 -b master https://github.com/openvswitch/ovs.git
-cd ovs;
-mkdir _gcc;
+# Get ovn source.
+git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
+cd ovn
 
-# build and install
+# Get OVS submodule, build and install OVS.
+git submodule update --init
+cd ovs
 ./boot.sh
-cd _gcc
-../configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
+./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
 --enable-ssl
-cd ..; make -C _gcc install; cd ..
-
+make -j8 install
+cd ..
 
-# get ovn source
-git clone --depth 1 -b $OVN_BRANCH $GITHUB_SRC
-cd ovn
-
-# build and install
+# Build and install OVN.
 ./boot.sh
 ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \
---enable-ssl --with-ovs-source=/build/ovs/ --with-ovs-build=/build/ovs/_gcc
-make -j8; make install
+--enable-ssl
+make -j8 install