Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
On 6/22/23 14:51, Dumitru Ceara wrote: > On 6/22/23 14:45, Dumitru Ceara wrote: >> On 6/22/23 13:56, Aaron Conole wrote: >>> Dumitru Ceara writes: >>> This avoids manual intervention when upstream ovn-kubernetes changes its dependency versions. Signed-off-by: Patryk Diak Co-authored-by: Patryk Diak Signed-off-by: Dumitru Ceara --- >>> >>> LGTM overall. It's also the only patch to have successfully built >>> recently, so I assume it will be important to apply for other patches to >>> succeed. >>> >> >> Thanks for the review! Yes, all other patches were failing ovn-kube CI >> when building the container image, effectively keeping us in the dark >> wrt effects on ovn-kubernetes. >> .ci/ovn-kubernetes/Dockerfile| 16 +++--- .ci/ovn-kubernetes/prepare.sh| 11 ++ .github/workflows/ovn-kubernetes.yml | 31 +--- >>> >>> We got some warnings on this file due to line lengths. Maybe we should >>> exclude yml from the line length check. WDYT? >>> >> >> Sure, I can post a patch for that. Should I prepare a patch for OVS >> though? We normally try to keep our checkpatch version in sync with the >> OVS one. >> 3 files changed, 39 insertions(+), 19 deletions(-) >>> >>> Acked-by: Aaron Conole >>> >> >> I pushed this patch to main. Hopefully the CI turns green soon. >> > > I forgot to mention: I added Patryk to the list of authors in AUTHORS.rst. > I went ahead and applied this patch to branches 23.06 and 23.03. It applies cleanly there and, at least, we can get signal on some ovn-kubernetes jobs there too. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
On 6/22/23 15:37, Aaron Conole wrote: > Dumitru Ceara writes: > >> On 6/22/23 13:56, Aaron Conole wrote: >>> Dumitru Ceara writes: >>> This avoids manual intervention when upstream ovn-kubernetes changes its dependency versions. Signed-off-by: Patryk Diak Co-authored-by: Patryk Diak Signed-off-by: Dumitru Ceara --- >>> >>> LGTM overall. It's also the only patch to have successfully built >>> recently, so I assume it will be important to apply for other patches to >>> succeed. >>> >> >> Thanks for the review! Yes, all other patches were failing ovn-kube CI >> when building the container image, effectively keeping us in the dark >> wrt effects on ovn-kubernetes. >> .ci/ovn-kubernetes/Dockerfile| 16 +++--- .ci/ovn-kubernetes/prepare.sh| 11 ++ .github/workflows/ovn-kubernetes.yml | 31 +--- >>> >>> We got some warnings on this file due to line lengths. Maybe we should >>> exclude yml from the line length check. WDYT? >>> >> >> Sure, I can post a patch for that. Should I prepare a patch for OVS >> though? We normally try to keep our checkpatch version in sync with the >> OVS one. > > Please do :) Then they can just be sync'd after, I guess. > Done: https://patchwork.ozlabs.org/project/openvswitch/patch/20230623121233.315243-1-dce...@redhat.com/ Thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
Dumitru Ceara writes: > On 6/22/23 13:56, Aaron Conole wrote: >> Dumitru Ceara writes: >> >>> This avoids manual intervention when upstream ovn-kubernetes changes >>> its dependency versions. >>> >>> Signed-off-by: Patryk Diak >>> Co-authored-by: Patryk Diak >>> Signed-off-by: Dumitru Ceara >>> --- >> >> LGTM overall. It's also the only patch to have successfully built >> recently, so I assume it will be important to apply for other patches to >> succeed. >> > > Thanks for the review! Yes, all other patches were failing ovn-kube CI > when building the container image, effectively keeping us in the dark > wrt effects on ovn-kubernetes. > >>> .ci/ovn-kubernetes/Dockerfile| 16 +++--- >>> .ci/ovn-kubernetes/prepare.sh| 11 ++ >>> .github/workflows/ovn-kubernetes.yml | 31 +--- >> >> We got some warnings on this file due to line lengths. Maybe we should >> exclude yml from the line length check. WDYT? >> > > Sure, I can post a patch for that. Should I prepare a patch for OVS > though? We normally try to keep our checkpatch version in sync with the > OVS one. Please do :) Then they can just be sync'd after, I guess. >>> 3 files changed, 39 insertions(+), 19 deletions(-) >> >> Acked-by: Aaron Conole >> > > I pushed this patch to main. Hopefully the CI turns green soon. > > Regards, > Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
On 6/22/23 14:45, Dumitru Ceara wrote: > On 6/22/23 13:56, Aaron Conole wrote: >> Dumitru Ceara writes: >> >>> This avoids manual intervention when upstream ovn-kubernetes changes >>> its dependency versions. >>> >>> Signed-off-by: Patryk Diak >>> Co-authored-by: Patryk Diak >>> Signed-off-by: Dumitru Ceara >>> --- >> >> LGTM overall. It's also the only patch to have successfully built >> recently, so I assume it will be important to apply for other patches to >> succeed. >> > > Thanks for the review! Yes, all other patches were failing ovn-kube CI > when building the container image, effectively keeping us in the dark > wrt effects on ovn-kubernetes. > >>> .ci/ovn-kubernetes/Dockerfile| 16 +++--- >>> .ci/ovn-kubernetes/prepare.sh| 11 ++ >>> .github/workflows/ovn-kubernetes.yml | 31 +--- >> >> We got some warnings on this file due to line lengths. Maybe we should >> exclude yml from the line length check. WDYT? >> > > Sure, I can post a patch for that. Should I prepare a patch for OVS > though? We normally try to keep our checkpatch version in sync with the > OVS one. > >>> 3 files changed, 39 insertions(+), 19 deletions(-) >> >> Acked-by: Aaron Conole >> > > I pushed this patch to main. Hopefully the CI turns green soon. > I forgot to mention: I added Patryk to the list of authors in AUTHORS.rst. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
On 6/22/23 13:56, Aaron Conole wrote: > Dumitru Ceara writes: > >> This avoids manual intervention when upstream ovn-kubernetes changes >> its dependency versions. >> >> Signed-off-by: Patryk Diak >> Co-authored-by: Patryk Diak >> Signed-off-by: Dumitru Ceara >> --- > > LGTM overall. It's also the only patch to have successfully built > recently, so I assume it will be important to apply for other patches to > succeed. > Thanks for the review! Yes, all other patches were failing ovn-kube CI when building the container image, effectively keeping us in the dark wrt effects on ovn-kubernetes. >> .ci/ovn-kubernetes/Dockerfile| 16 +++--- >> .ci/ovn-kubernetes/prepare.sh| 11 ++ >> .github/workflows/ovn-kubernetes.yml | 31 +--- > > We got some warnings on this file due to line lengths. Maybe we should > exclude yml from the line length check. WDYT? > Sure, I can post a patch for that. Should I prepare a patch for OVS though? We normally try to keep our checkpatch version in sync with the OVS one. >> 3 files changed, 39 insertions(+), 19 deletions(-) > > Acked-by: Aaron Conole > I pushed this patch to main. Hopefully the CI turns green soon. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
Dumitru Ceara writes: > This avoids manual intervention when upstream ovn-kubernetes changes > its dependency versions. > > Signed-off-by: Patryk Diak > Co-authored-by: Patryk Diak > Signed-off-by: Dumitru Ceara > --- LGTM overall. It's also the only patch to have successfully built recently, so I assume it will be important to apply for other patches to succeed. > .ci/ovn-kubernetes/Dockerfile| 16 +++--- > .ci/ovn-kubernetes/prepare.sh| 11 ++ > .github/workflows/ovn-kubernetes.yml | 31 +--- We got some warnings on this file due to line lengths. Maybe we should exclude yml from the line length check. WDYT? > 3 files changed, 39 insertions(+), 19 deletions(-) Acked-by: Aaron Conole ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
Bleep bloop. Greetings Dumitru Ceara, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 87 characters long (recommended limit is 79) #116 FILE: .github/workflows/ovn-kubernetes.yml:47: .ci/ovn-kubernetes/prepare.sh src/github.com/ovn-org/ovn-kubernetes $GITHUB_ENV WARNING: Line is 87 characters long (recommended limit is 79) #144 FILE: .github/workflows/ovn-kubernetes.yml:112: .ci/ovn-kubernetes/prepare.sh src/github.com/ovn-org/ovn-kubernetes $GITHUB_ENV Lines checked: 160, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Figure out dependencies dynamically.
This avoids manual intervention when upstream ovn-kubernetes changes its dependency versions. Signed-off-by: Patryk Diak Co-authored-by: Patryk Diak Signed-off-by: Dumitru Ceara --- .ci/ovn-kubernetes/Dockerfile| 16 +++--- .ci/ovn-kubernetes/prepare.sh| 11 ++ .github/workflows/ovn-kubernetes.yml | 31 +--- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/.ci/ovn-kubernetes/Dockerfile b/.ci/ovn-kubernetes/Dockerfile index 01be205429..12f8190172 100644 --- a/.ci/ovn-kubernetes/Dockerfile +++ b/.ci/ovn-kubernetes/Dockerfile @@ -1,5 +1,5 @@ ARG OVNKUBE_COMMIT=master -ARG LIBOVSDB_COMMIT=a6a173993830 +ARG GO_VERSION FROM fedora:37 AS ovnbuilder @@ -34,12 +34,9 @@ RUN rm rpm/rpmbuild/RPMS/x86_64/*debug* RUN rm rpm/rpmbuild/RPMS/x86_64/*docker* # Build ovn-kubernetes -FROM golang:1.18 as ovnkubebuilder +ARG GO_VERSION +FROM golang:$GO_VERSION as ovnkubebuilder ARG OVNKUBE_COMMIT -ARG LIBOVSDB_COMMIT - -# Get a working version of libovsdb (for modelgen). -RUN GO111MODULE=on go install github.com/ovn-org/libovsdb/cmd/modelgen@${LIBOVSDB_COMMIT} # Clone OVN Kubernetes and build the binary based on the commit passed as argument WORKDIR /root @@ -52,9 +49,14 @@ RUN git checkout ${OVNKUBE_COMMIT} && git log -n 1 RUN mkdir -p /tmp/ovn/.ci/ovn-kubernetes COPY .ci/ovn-kubernetes /tmp/ovn/.ci/ovn-kubernetes WORKDIR /tmp/ovn -RUN .ci/ovn-kubernetes/prepare.sh /root/ovn-kubernetes +RUN .ci/ovn-kubernetes/prepare.sh /root/ovn-kubernetes /dev/null WORKDIR /root/ovn-kubernetes/go-controller +# Get a working version of libovsdb (for modelgen). +RUN GO111MODULE=on go install github.com/ovn-org/libovsdb/cmd/modelgen@$( \ +go list -mod=mod -m -f '{{ .Version }}' github.com/ovn-org/libovsdb \ +) + # Make sure we use the OVN NB/SB schema from the local code. COPY --from=ovnbuilder /tmp/ovn/ovn-nb.ovsschema pkg/nbdb/ovn-nb.ovsschema COPY --from=ovnbuilder /tmp/ovn/ovn-sb.ovsschema pkg/sbdb/ovn-sb.ovsschema diff --git a/.ci/ovn-kubernetes/prepare.sh b/.ci/ovn-kubernetes/prepare.sh index 8fc9652afd..e0cc722ede 100755 --- a/.ci/ovn-kubernetes/prepare.sh +++ b/.ci/ovn-kubernetes/prepare.sh @@ -3,13 +3,24 @@ set -ev ovnk8s_path=$1 +env_path=$2 topdir=$PWD +function extract_ci_var() { +local name=$1 + +grep "$name:" .github/workflows/test.yml | awk '{print $2}' | tr -d '"' +} + pushd ${ovnk8s_path} # Add here any custom operations that need to performed on the # ovn-kubernetes cloned repo, e.g., custom patches. +# Set up the right GO_VERSION and K8S_VERSION. +echo "GO_VERSION=$(extract_ci_var GO_VERSION)" >> $env_path +echo "K8S_VERSION=$(extract_ci_var K8S_VERSION)" >> $env_path + # git apply --allow-empty is too new so not all git versions from major # distros support it, just check if the custom patch file is not empty # before applying it. diff --git a/.github/workflows/ovn-kubernetes.yml b/.github/workflows/ovn-kubernetes.yml index b8cacf873e..36d2db8492 100644 --- a/.github/workflows/ovn-kubernetes.yml +++ b/.github/workflows/ovn-kubernetes.yml @@ -13,10 +13,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.18.4" - K8S_VERSION: v1.24.0 OVNKUBE_COMMIT: "master" - LIBOVSDB_COMMIT: "a6a173993830" KIND_CLUSTER_NAME: ovn KIND_INSTALL_INGRESS: true KIND_ALLOW_SYSTEM_WRITES: true @@ -39,10 +36,20 @@ jobs: with: submodules: recursive +- name: Check out ovn-kubernetes + uses: actions/checkout@v3 + with: + path: src/github.com/ovn-org/ovn-kubernetes + repository: ovn-org/ovn-kubernetes + +- name: Prepare + run: | +.ci/ovn-kubernetes/prepare.sh src/github.com/ovn-org/ovn-kubernetes $GITHUB_ENV + - name: Build ovn-kubernetes container run: | docker build --build-arg OVNKUBE_COMMIT=${{ env.OVNKUBE_COMMIT }} \ - --build-arg LIBOVSDB_COMMIT=${{ env.LIBOVSDB_COMMIT }} \ + --build-arg GO_VERSION=${{ env.GO_VERSION }} \ --squash -t ovn-daemonset-f:dev -f .ci/ovn-kubernetes/Dockerfile . mkdir /tmp/_output docker save ovn-daemonset-f:dev > /tmp/_output/image.tar @@ -91,12 +98,6 @@ jobs: - name: Free up disk space run: sudo eatmydata apt-get remove --auto-remove -y aspnetcore-* dotnet-* libmono-* mono-* msbuild php-* php7* ghc-* zulu-* -- name: Set up Go - uses: actions/setup-go@v3 - with: -go-version: ${{ env.GO_VERSION }} - id: go - - name: Check out ovn uses: actions/checkout@v3 @@ -108,9 +109,15 @@ jobs: - name: Prepare run: | -.ci/ovn-kubernetes/prepare.sh src/github.com/ovn-org/ovn-kubernetes +.ci/ovn-kubernetes/prepare.sh src/github.com/ovn-org/ovn-kubernetes $GITHUB_ENV + +- name: Set up Go + uses: actions/setup-go@v3 + with: +go-version: ${{ env.GO_VERSION }} + id: go -- name: Set up environment +- name: Set up GOPATH