search

ovn-org / ovn-kubernetes

A robust Kubernetes networking platform
https://ovn-kubernetes.io/
Apache License 2.0
767 stars 333 forks source link

Fix the Fedora dev Dockerfile. #4448

Closed dceara closed 1 week ago

dceara commented 1 week ago

What this PR does and why is it needed

This PR fixes the dev Dockerfile. It's needed in order to be able to easily test custom OVN changes.

Description for the changelog

Fix OVS/OVN build steps in Dockerfile.fedora.dev.

Does this PR introduce a user-facing change?

NONE
coveralls commented 1 week ago

Coverage Status

coverage: 52.718% (+0.002%) from 52.716% when pulling bc8eec1331d140f075153dc6457ac390996d7a2d on dceara:fix-dockerfile-dev into 701b8e077c6d5639c45ad5cc41fbd4d8759ddc56 on ovn-org:master.

dceara commented 1 week ago

main changes lgtm now that we are mandating docs, could you please add something to https://ovn-kubernetes.io/developer-guide/local_testing_guide/ (currently this has nothing sorry about that, but please add a subsection that says "Testing custom OVN and OVS Fixes" or something -> nothing too extravagant it would be good to document that we actually do have a dev dockerfile just for this and here is how you can use this guys!!!)

I could but, to be completely fair, I think the right way to do this is to open an issue (or more because as you point out there's nothing at all about testing locally) instead of adding documentation unrelated to the change (which is to fix the Dockerfile itself).

Another reason for doing that as a separate issue is that I'd have to add a subsection that talks about starting kind, e.g., kind.sh --ovn-image <IMAGE> but then there's nothing at all about how to use kind in general. I wouldn't be surprised if whenever the rest of the local_testing_guide is written the subsection I'd be writing now would have to change.

tssurya commented 1 week ago

I could but, to be completely fair, I think the right way to do this is to open an issue (or more because as you point out there's nothing at all about testing locally) instead of adding documentation unrelated to the change (which is to fix the Dockerfile itself).

not asking for writing the entire doc, only the piece you have modified currently which is how to easily run custom ovn/ovs changes in KIND.

Another reason for doing that as a separate issue is that I'd have to add a subsection that talks about starting kind, e.g., kind.sh --ovn-image <IMAGE> but then there's nothing at all about how to use kind in general. I wouldn't be surprised if whenever the rest of the local_testing_guide is written the subsection I'd be writing now would have to change.

https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-on-kind/ we already have it here, you can just start saying

"Testing custom OVN and OVS Fixes"

"See here for details on KIND installation. In order to run custom changes in OVN/OVS here are the steps:

  1. make the change in OVN and get it merged into OVN main?
  2. build image with this dockerfile.dev which is meant for delvelopmental testing of ovs and ovn?
  3. this pulls from main branch hence tada
  4. you can now create a kind cluster with this docker image
  5. check your OVN version on this cluster and you can see its the latest that's all I meant like 10lines. => please don't add any docs that is not related to your change

FWIW the reason I am asking for docs in PRs is I have tried the "let's open issues approach" and nobody picks it up and we both know it won't happen (so far its just been me doing stuff because docs never get prioritized; so don't bother writing the missing bits or history at all, just need a skeleton for your specific change (assume you have everything else already there) and some day I will wrap around it)

dceara commented 1 week ago

Thanks @tssurya!