weave icon indicating copy to clipboard operation
weave copied to clipboard

IPALLOC_RANGE improvement

Open tsunamireversal-zz opened this issue 5 years ago • 11 comments

Is this a FEATURE REQUEST? yes, an improvement to weave.yaml to allow automation software to easily find the tag to replace instead of trying to parse the weave.yaml file. I did not find a similar issue.

why: As per https://www.weave.works/docs/net/latest/kubernetes/kube-addon/#configuration-options in the section "Manually editing the YAML file", we unfortunately have to set the IPALLOC_RANGE flag for many deployments, especially clouds like AWS and Azure, and/or whenever we hit a 10.x.x.x network (which is often). If left untouched, the host network clashes with the kubernetes (weave) one

what: What I would really like to see is either one of those two things to automate and simplify the flow:

  • Option 1 Add a comment just above where we would see IPALLOC_RANGE appear in, so it's findable by sed or ansible lineinfile module, such as:

    containers:
      - name: weave
        env:
          ### IPALLOC_RANGE
    
  • Option 2 Force it by default inside the file instead of making it optional, with an easy to recognize/modify subnet for instance

    containers:
      - name: weave
        env:
          - name: IPALLOC_RANGE
            value: 172.26.0.0/16
    

To allow automation to replace it easily before applying it to a kubernetes master. Also I believe a 172.26 (would still be a 16 bit network mask) and it would not clash and could be applied to almost any configuration vs the current default of 10.x.x.x.

NOTE: Obviously, it would still be possible to override it using the cli flag if one needed to do so but it would also work correctly as per https://www.weave.works/blog/weave-net-kubernetes-integration by putting a change in place.

  • applicability I believe this new feature should apply to all versions, unless I don't see something?

tsunamireversal-zz avatar Jun 16 '20 21:06 tsunamireversal-zz

EDIT: Option 1 Sounds reasonable. Have you tried using a tool which is aimed at machine-editing yaml files like this, e.g. Kustomize?

bboreham avatar Jun 17 '20 08:06 bboreham

Yes, I've used kustomize before, although not for this particular use case. I'd say not everyone uses kustomize (or any other tool that would solve this), as it introduces some new workflow and/or dependency to another tool in the equation.

I can continue and try to solve this using a parser, but really enjoyed the "it just works out-of-the-box" aspect which would work for (I believe) all network types and clouds if the file was fixed at the source

tsunamireversal-zz avatar Jun 17 '20 13:06 tsunamireversal-zz

I believe a 172.26 (would still be a 16 bit network mask)

So 1/16th the size of the Weave Net default.

and it would not clash

Sadly, this is extremely unlikely. It will clash with something.

This is exactly how we picked 10.32/12: we surveyed every cloud we had access to, and picked a range that didn't clash with any of them.

We can't change the default now: it would break everyone on upgrade.

bboreham avatar Jun 18 '20 15:06 bboreham

I see, yeah, we don't want that, I wrongly assumed it was a 16 bit network by default on the 10.x.x.x interface.

That said, there can still be an anchor (commented) that would make it easy to add the IPALLOC_RANGE tag on the source file? That shouldn't break anything normally

Could even be something that looks standardized like in many linux config files where they comment out the string so you see it's possible and what the default is, like

containers:
  - name: weave
    env:
#      - name: IPALLOC_RANGE
#        value: 10.32.0.0/12

Or, since it's the default anyways, we could just have it appear there and then it's easy to adjust

containers:
  - name: weave
    env:
      - name: IPALLOC_RANGE
        value: 10.32.0.0/12

Would this be a valid option?

tsunamireversal-zz avatar Jun 19 '20 13:06 tsunamireversal-zz

Any updates?

tsunamireversal-zz avatar Jul 03 '20 00:07 tsunamireversal-zz

Yes, adding a comment or the default to the yaml sounds reasonable.

bboreham avatar Jul 03 '20 08:07 bboreham

Hi! Just wondering... has it been implemented?

Thanks

tsunamireversal-zz avatar Jul 31 '20 18:07 tsunamireversal-zz

Nobody has contributed this change. Would you like to do it?

bboreham avatar Jul 31 '20 18:07 bboreham

Sure! I'd like to send a contribution, but isn't this weave.yaml located inside the website cloud.weave.works itself? I tried to sift through the codebase here, but couldn't find where the weave.yaml template found on the website is located. Any ideas?

I found this:

./prog/weave-kube/weave-daemonset-k8s-1.11.yaml
./prog/weave-kube/weave-daemonset-k8s-1.6.yaml
./prog/weave-kube/weave-daemonset-k8s-1.8.yaml
./prog/weave-kube/weave-daemonset-k8s-1.7.yaml
./prog/weave-kube/weave-daemonset-k8s-1.9.yaml

But not sure if it'll have any impact if I were to modify those.

I can grab the file if I wget/curl this, but modifying some master file on cloud.weave.works, I'm not sure.

https://cloud.weave.works/k8s/net?k8s-version=$(kubectl version | base64 | tr -d '\n')

tsunamireversal-zz avatar Aug 24 '20 17:08 tsunamireversal-zz

Any suggestions?

tsunamireversal-zz avatar Sep 23 '20 22:09 tsunamireversal-zz

Any pointers would be appreciated!

tsunamireversal-zz avatar Nov 10 '20 19:11 tsunamireversal-zz