opened
serbrech
username_0
**What this PR does / why we need it**
fixes #2553. Bypasses the limit of 256 variables in a deployment.
Allows to declare more agent pools.

- [x]  move the agentPoolProfile variables into its own json object.
- [x] adapt all `variable()` references in the k8s arm templates
- [x]  test backward compatibility for k8s (ie. deploy with previous version, upgrade with this branch)

TODO : 
- [ ]  review go code to make sure it does not reference any agent pool variables from go
- [ ] adapt all `variable()` references in the dcos arm templates
- [ ] adapt all `variable()` references in the swarnarm templates

Note : 
Helpful regex to find all occurences of a variable in the go templates (in this specific case but can be adjusted for future work)

find `(variables\('\{\{\.Name\}\})([a-zA-Z]*)'\)` and replace with `$1Config').$2` 

For example, the above finds and replaces occurences of 

`variables('{{.Name}}SomeProperty')` 
with   
`variables('{{.Name}}Config').SomeProperty`
which is helpful when moving SomeProperty one level down into a json object.
[WIP] : Moving agent pool variable in its own json object
issue
created
serbrech
username_0
hm.. just tried a deployment and the cluster deploys and answers correctly.
How reliable are the CircleCI e2e tests?
comment
created
serbrech
username_0
It seems that there is a maintainability issue that starts to show : 
- The variables defined in `kubernetesagentvars.t` are referenced from go  (`engine.go` for example) to build parts of the template through the FunctionMap. This makes the template very hard to change because it is hard to find where a variable is used in the code.
- The `engine.go` and the `FunctionMap` are common to all orchestrator types. this makes it hard to know whether a function is used for `dcos`, `kubernetes`, `swarm`, or a combination of them all.

For example : 
`getVNETSubnetDependencies` is using the agent pool profile variables to construct parts of the arm templates. it is available to all orchestrator templates, and it relies on the agentpoolprofile variable structure, which could be different across orchestrators.

This emphasizes the need to separate the orchestrator specific code. f.ex, provide an orchestrator specific function map and make the orchestrator specific code explicit and visible.
comment
created
CecileRobertMichon
username_1
@username_0 let me know when this is ready for review
comment
created
serbrech
username_0
@username_2 @username_1 
I have been unable to dedicate time to this since my initial PR.
If anyone want to pick this up, I can assist sharing what I know, but the above comment gave a good insight on the scope of the task.

Here is some of my thoughts on the task : 

The work spans all orchestrators. There are references to the variable names throughout the go files as shown above, which makes this a very tedious and bug-prone task.

Suggestion : 
- Duplicate this file to avoid reusing the template files across orchestrators to reduce the risks and impact during the refactoring.
- instead of moving **all** the variables in a new object parameter at once, create an empty object and move the variables one by one, splitting the scope of this work into multiple PR. This will make the template a little messy in the transition, but will reduce conflict and reduce the number of variable earlier for users.
- Consider splitting this template in multiple files to reduce conflicts. it is being updated too often.

Any opinion on this?
comment
created
jackfrancis
username_2
From experiments good things will arise. :)

Closing this, the conversations in here will provide some good background when we re-engage on how to make this better (use data model only and not go templating, etc)
comment