Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Create a --set flag for generate #2787

Merged
merged 12 commits into from
May 2, 2018

Conversation

jcorioland
Copy link
Contributor

@jcorioland jcorioland commented Apr 26, 2018

What this PR does / why we need it: this PR adds support for a --flag to the generate command to make possible to override values from the api model JSON file using the CLI:

acs-engine generate --set linuxProfile.adminUsername=jcorioland,agentPoolProfiles[0].count=5 kubernetes.json

Which issue this PR fixes: fixes #733

Special notes for your reviewer:

It supports all fields and arrays under the properties node.
It introduces some refactoring of the generate command itself and new unit tests.
It adds a dependency to Jeffail/gabs for JSON manipulations.

If applicable:

  • documentation
  • unit tests

@jcorioland jcorioland self-assigned this Apr 26, 2018
@ghost ghost added the in progress label Apr 26, 2018
@codecov
Copy link

codecov bot commented Apr 27, 2018

Codecov Report

Merging #2787 into master will increase coverage by 0.12%.
The diff coverage is 68.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2787      +/-   ##
==========================================
+ Coverage   46.95%   47.07%   +0.12%     
==========================================
  Files          86       87       +1     
  Lines       12781    12858      +77     
==========================================
+ Hits         6001     6053      +52     
- Misses       6226     6240      +14     
- Partials      554      565      +11
Impacted Files Coverage Δ
cmd/generate.go 32.38% <65%> (+5.94%) ⬆️
pkg/acsengine/transform/apimodel_merger.go 69.49% <69.49%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b7cef8...8cdd0de. Read the comment docs.

@jcorioland jcorioland changed the title WIP: Create a --set flag for generate Create a --set flag for generate Apr 27, 2018
@CecileRobertMichon CecileRobertMichon changed the title Create a --set flag for generate [WIP] Create a --set flag for generate Apr 27, 2018
@jcorioland jcorioland changed the title [WIP] Create a --set flag for generate Create a --set flag for generate Apr 27, 2018
@CecileRobertMichon
Copy link
Contributor

@jcorioland did you decide to add the --set flag to deploy in a separate PR?

@jcorioland
Copy link
Contributor Author

@CecileRobertMichon yes, I will do that in another PR as it may lead to a some refactoring to share code between generate and deploy command. I've created an issue (#2796) to work on the deploy command later.

@CecileRobertMichon CecileRobertMichon added the cse-sync-week Triage for issues that would be good for CSE sync week, April 24-27th 2018 label Apr 30, 2018
Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm pending E2E, thanks @jcorioland!!

@jackfrancis jackfrancis merged commit ca9b3f1 into Azure:master May 2, 2018
@ghost ghost removed the in progress label May 2, 2018
@jcorioland jcorioland deleted the jcorioland/set-flag branch May 3, 2018 07:39
@burn2delete
Copy link

@jcorioland can you confirm this works for setting the linuxProfile.ssh.publicKeys[0].keyData='rsa ABC...

I am getting the following output:

acs-engine generate --api-model ./config/develop.json --set servicePrincipalProfile.clientID=test,servicePrincipalProfile.secret=test,linuxProfile.ssh.publicKeys[0].keyData="test" 
INFO[0000] --set flag array value detected. Name: linuxProfile.ssh.publicKeys, Index: 0, PropertyName: keyData 
INFO[0000] new api model file has been generated during merge: /tmp/mergedApiModel136504300 
INFO[0000] Error returned by LoadContainerService: Namespace Properties.LinuxProfile.SSH.PublicKeys is not caught, Key: 'Properties.LinuxProfile.SSH.PublicKeys' Error:Field validation for 'PublicKeys' failed on the 'required' tag 
FATA[0000] error loading API model in generateCmd: error parsing the api model: Namespace Properties.LinuxProfile.SSH.PublicKeys is not caught, Key: 'Properties.LinuxProfile.SSH.PublicKeys' Error:Field validation for 'PublicKeys' failed on the 'required' tag 

@jcorioland
Copy link
Contributor Author

jcorioland commented May 31, 2018

@flyboarder Ah, this is weird. I've just tested with a sample json api model file and it seems to work:

./bin/acs-engine generate --api-model ./pkg/acsengine/testdata/simple/kubernetes.json --set servicePrincipalProfile.clientID=test,servicePrincipalProfile.secret=test,linuxProfile.ssh.publicKeys[0].keyData="test"
INFO[0000] --set flag array value detected. Name: linuxProfile.ssh.publicKeys, Index: 0, PropertyName: keyData
INFO[0000] new api model file has been generated during merge: /tmp/mergedApiModel819228615
INFO[0000] Generating assets into _output/masterdns1...

Can you check the tmp file that is generated? In mine, I can see that the "test" is there for keyData:

root@e52276aaed30:/gopath/src/github.com/Azure/acs-engine# cat /tmp/mergedApiModel819228615
{
	"apiVersion": "vlabs",
	"properties": {
		"agentPoolProfiles": [
			{
				"availabilityProfile": "AvailabilitySet",
				"count": 3,
				"name": "agentpool1",
				"vmSize": "Standard_D2_v2"
			},
			{
				"availabilityProfile": "AvailabilitySet",
				"count": 3,
				"name": "agentpool2",
				"vmSize": "Standard_D2_v2"
			}
		],
		"certificateProfile": {
			"apiServerCertificate": "apiServerCertificate",
			"apiServerPrivateKey": "apiServerPrivateKey",
			"caCertificate": "caCertificate",
			"caPrivateKey": "caPrivateKey",
			"clientCertificate": "clientCertificate",
			"clientPrivateKey": "clientPrivateKey",
			"etcdClientCertificate": "etcdClientCertificate",
			"etcdClientPrivateKey": "etcdClientPrivateKey",
			"etcdPeerCertificates": [ "etcdPeerCertificate0" ],
			"etcdPeerPrivateKeys": [ "etcdPeerPrivateKey0" ],
			"etcdServerCertificate": "etcdServerCertificate",
			"etcdServerPrivateKey": "etcdServerPrivateKey",
			"kubeConfigCertificate": "kubeConfigCertificate",
			"kubeConfigPrivateKey": "kubeConfigPrivateKey"
		},
		"linuxProfile": {
			"adminUsername": "azureuser",
			"ssh": { "publicKeys": [ { "keyData": "test" } ] }
		},
		"masterProfile": {
			"count": 1,
			"dnsPrefix": "masterdns1",
			"vmSize": "Standard_D2_v2"
		},
		"orchestratorProfile": { "orchestratorType": "Kubernetes" },
		"servicePrincipalProfile": {
			"clientID": "test",
			"clientId": "ServicePrincipalClientID",
			"secret": "test"
		}
	}
}

Can you either share your api model file or try with the sample I've used please ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting review cse-sync-week Triage for issues that would be good for CSE sync week, April 24-27th 2018
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a --set flag for generate
4 participants