initial outline of the acs template generator #3
Conversation
|
Out of curiosity, what's the difference between putting stuff here versus in VSO? |
|
this tool will initially be for customers to build their custom templates |
| } | ||
|
|
||
| func (t *TemplateOutputs) generateContent(acsCluster *vlabs.AcsCluster) error { | ||
| t.Master = "TemplateOutputs master" |
There was a problem hiding this comment.
This is a fake string, I think, right? Eventually to be replaced with a full JSON object?
How do you anticipate actually loading in the relevant data here?
There was a problem hiding this comment.
This method is gone, and the acsCluster object is bound directly to the templates.
| Agents string | ||
| // Diagnostics describes the diagnostics section | ||
| Diagnostics string | ||
| } |
There was a problem hiding this comment.
These represent the Master and Agent FQDN and Diagnostics Storage Account Uri like the current template in ACS? If yes, then you should rename the fields to match the template output so that the names are descriptive.
There was a problem hiding this comment.
These sections are gone. I've made the model under api/vlabs closely match closely to our API model.
| "isStateless": false | ||
| }, | ||
| { | ||
| "name": "agentpools", |
There was a problem hiding this comment.
Agent pool names should be unique within the cluster so you should name them as such in the example as well.
There was a problem hiding this comment.
thanks, I've added validation to catch this now.
| // DCOS is the string constant for DCOS orchestrator type | ||
| DCOS = "DCOS" | ||
| // SWARM is the string constant for the Swarm orchestrator type | ||
| SWARM = "Swarm" |
There was a problem hiding this comment.
Why all caps for Swarm?
| } | ||
|
|
||
| // AgentPoolProfiles represents an agent pool definition | ||
| type AgentPoolProfiles struct { |
There was a problem hiding this comment.
This should be: 'AgentPoolProfile' (singular).
| Count int `json:"count"` | ||
| VMSize string `json:"vmSize"` | ||
| IsStateless bool `json:"isStateless,omitempty"` | ||
| DNSPrefix string `json:"dnsPrefix,omitempty"` |
There was a problem hiding this comment.
This is a required field. Why would be it empty?
There was a problem hiding this comment.
validation now added to require this if the user specifies ports
| {{.Variables.Agents}} | ||
| {{.Variables.Diagnostics}} | ||
| }, | ||
| "resources": [ |
There was a problem hiding this comment.
Division of resources in 3 parts might be limiting since there will be common resources/vars between masters and agents like OS image pub/offer/sku.
There was a problem hiding this comment.
this has been heavily refactored from this initial version
|
Is this ready to go? I'm happy to merge and iterate. |
|
Yes, this is ready to go. Please integrate. |
There was a problem hiding this comment.
I didn't take the time to groc how different the swarm and disk templates are. But offhand it looks like we might be able to push those two templates file groups into one template files group, or we could break the shared pieces into template files that both call so that we wouldn't need as much duplication.
| {"outputs", "zoutputs"}, | ||
| } | ||
|
|
||
| template = translateJSON(template, translateParams, false) |
There was a problem hiding this comment.
This feels like a bug waiting to happen where a client names something verbedparameterstore, or something of the sort and we rename things to verbeparameterstore. Is there a way we can introduce path to the values we are changing
There was a problem hiding this comment.
I shared this offline and anhowe fixed it by adding the quotes. Still possible, but Much less likely.
| @@ -0,0 +1,392 @@ | |||
| package clustertemplate | |||
There was a problem hiding this comment.
How some of the Json generation logic is here and some is in the go template files looks confusing to me. Is there a clean way to take the parts here where we are making json and move them in to the template files?
There was a problem hiding this comment.
there is a strike of balance between having too many levels of templatefiles. Brendan had suggested having everything in code using JSON Path. I'm still trying to figure this out.
Parametrize kubernetes version checking function
update Heapster CR and CRB
make openshift orchestrator output ARM template from azure-ansible 3.7 refarch
No description provided.