chore: reduce customData payload #3793

jackfrancis · 2020-09-04T20:12:26Z

Reason for Change:

Yes, this is a chore.

Reducing customData (Azure ARM template cloud-init payload) via:

moved the static "generate proxy certs" script to the VHD, so that VHD-enabled cluster buildouts don't need to include that script at runtime
a CSE once-over, to shorten variable names, remove duplication, and other optimizations that have no functional impact but reduce the payload of the script itself

Issue Fixed:

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Notes:

codecov · 2020-09-04T20:26:01Z

Codecov Report

Merging #3793 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3793      +/-   ##
==========================================
- Coverage   73.21%   73.19%   -0.03%     
==========================================
  Files         148      148              
  Lines       25385    25394       +9     
==========================================
+ Hits        18586    18587       +1     
- Misses       5663     5671       +8     
  Partials     1136     1136

Impacted Files	Coverage Δ
pkg/engine/templates_generated.go	`53.42% <ø> (ø)`
pkg/api/types.go	`94.32% <100.00%> (ø)`
pkg/engine/armvariables.go	`86.49% <100.00%> (ø)`
cmd/upgrade.go	`42.71% <0.00%> (-1.50%)`	⬇️

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 c58ad50...fe3f49b. Read the comment docs.

mboersma

/lgtm

Just some optional suggestions.

mboersma · 2020-09-08T20:29:11Z

examples/e2e-tests/kubernetes/release/default/definition.json

@@ -50,20 +50,21 @@
          },
          {
            "name": "node-problem-detector",
-            "enabled": true
+            "enabled": false


😢 oh well.

this is just temporary :)

mboersma · 2020-09-08T20:30:44Z

parts/k8s/cloud-init/artifacts/cse_config.sh

-  ETCD_SERVER_PRIVATE_KEY_PATH="/etc/kubernetes/certs/etcdserver.key"
-  touch "${ETCD_SERVER_PRIVATE_KEY_PATH}"
+  local apiserver_key="/etc/kubernetes/certs/apiserver.key" ca_key="/etc/kubernetes/certs/ca.key" etcdserver_key="/etc/kubernetes/certs/etcdserver.key"
+  touch "${apiserver_key}"


touch takes multiple arguments, so these three commands could be a one-liner for a savings of...drumroll...12 bytes.

mboersma · 2020-09-08T20:31:57Z

parts/k8s/cloud-init/artifacts/cse_config.sh

-  ETCD_PEER_PRIVATE_KEY_PATH="/etc/kubernetes/certs/etcdpeer${NODE_INDEX}.key"
-  touch "${ETCD_PEER_PRIVATE_KEY_PATH}"
+  local etcdclient_key="/etc/kubernetes/certs/etcdclient.key" etcdpeer_key="/etc/kubernetes/certs/etcdpeer${NODE_INDEX}.key"
+  touch "${etcdclient_key}"


Again, this could be

touch "${etcdclient_key}" "${etcdpeer_key}"

and so on below...

mboersma · 2020-09-08T20:34:08Z

parts/k8s/cloud-init/artifacts/cse_config.sh

-  echo "${ETCD_SERVER_CERTIFICATE}" | base64 --decode >"${ETCD_SERVER_CERTIFICATE_PATH}"
-  echo "${ETCD_CLIENT_CERTIFICATE}" | base64 --decode >"${ETCD_CLIENT_CERTIFICATE_PATH}"
-  echo "${ETCD_PEER_CERT}" | base64 --decode >"${ETCD_PEER_CERTIFICATE_PATH}"
+  echo "${APISERVER_PRIVATE_KEY}" | base64 --decode >"${apiserver_key}"


base64 -d could save some bytes if we're ok with the short argument style.

mboersma · 2020-09-08T20:42:22Z

parts/k8s/cloud-init/artifacts/cse_config.sh

-    dkms remove -m nvidia -v $GPU_DV -k $KERNEL_NAME
+  local d="/var/lib/dkms/nvidia/${GPU_DV}" k log_file="/var/log/nvidia-installer-$(date +%s).log"
+  k=$(uname -r)
+  if [ -d $d ]; then


Shortening these variable names makes things less readable, but these changes are mostly localized so I suppose it's tolerable. Whatever is necessary to save bytes.

mboersma · 2020-09-08T20:46:19Z

parts/k8s/cloud-init/artifacts/cse_install.sh

-    local path="/usr/bin"
-    CONTAINER_IMAGE=${ETCD_DOWNLOAD_URL}etcd:v${ETCD_VERSION}
-    pullContainerImage $CLI_TOOL ${CONTAINER_IMAGE}
+  local  v


Extra space here. 🙀

parts/k8s/cloud-init/artifacts/cse_install.sh

acs-bot · 2020-09-08T21:02:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, mboersma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jackfrancis,mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jackfrancis · 2020-09-08T21:10:00Z

Hold merge until we get a new VHD built and merged from this commit, so that we can undo the E2E cluster config changes.

acs-bot · 2020-09-09T20:09:07Z

New changes are detected. LGTM label has been removed.

jackfrancis · 2020-09-10T01:09:10Z

Flatcar config regressed:

Error from deployment for kubernetes-westus2-29909 in resource group kubernetes-westus2-29909:exit status 1
 2020/09/10 00:30:51 Command Output: Deployment failed. Correlation ID: bcee57e6-b586-4d8f-89a2-d057062e6da0. {
   "status": "Failed",
   "error": {
     "code": "ResourceDeploymentFailure",
     "message": "The resource operation completed with terminal provisioning state 'Failed'.",
     "details": [
       {
         "code": "VMExtensionProvisioningError",
         "message": "VM has reported a failure when processing extension 'vmssCSE'. Error message: \"Enable failed: failed to execute command: command terminated with exit status=6\n[stdout]\nThu Sep 10 00:08:59 UTC 2020,k8s-pool1-13077327-vmss000000\n\n[stderr]\ngrep: /opt/azure/containers/provision.sh: No such file or directory\ngrep: /opt/azure/containers/provision.sh: No such file or directory\ngrep: /opt/azure/containers/provision.sh: No such file or directory\ngrep: /opt/azure/containers/provision.sh: No such file or directory\n\"\r\n\r\nMore information on troubleshooting is available at https://aka.ms/VMExtensionCSELinuxTroubleshoot "
       }
     ]
   }
 }

acs-bot added size/XL approved labels Sep 4, 2020

jackfrancis force-pushed the customData-reduce branch from b77d611 to 62ed2cf Compare September 4, 2020 23:28

mboersma previously approved these changes Sep 8, 2020

View reviewed changes

acs-bot assigned mboersma Sep 8, 2020

acs-bot added the lgtm label Sep 8, 2020

jackfrancis added the do-not-merge/hold label Sep 8, 2020

jackfrancis dismissed mboersma’s stale review via 41b57cd September 9, 2020 20:09

jackfrancis force-pushed the customData-reduce branch from 09f4fe6 to 41b57cd Compare September 9, 2020 20:09

acs-bot removed the lgtm label Sep 9, 2020

jackfrancis added 11 commits September 9, 2020 16:27

chore: reduce customData payload

a33bff4

try to make VHD CI happy

7dd1bab

chore: generated code

992a71b

try this

31e1670

this should work

7307359

more unbound vars

a0aeeee

another local var audit

460f42c

fix and allow to be PR tested

35718ff

we don't need this

661bdea

undo cluster config changes w/ new VHD

a908825

undo this change

802b5c7

jackfrancis force-pushed the customData-reduce branch from 41b57cd to 802b5c7 Compare September 9, 2020 23:58

fix flatcar

fe3f49b

jackfrancis merged commit db44f60 into Azure:master Sep 10, 2020

jackfrancis deleted the customData-reduce branch September 10, 2020 19:45

fmotrifork mentioned this pull request Sep 21, 2020

aks-engine 0.56.0 fishworks/fish-food#964

Merged

penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020

chore: reduce customData payload (Azure#3793)

2d9f1fd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: reduce customData payload #3793

chore: reduce customData payload #3793

jackfrancis commented Sep 4, 2020

codecov bot commented Sep 4, 2020 •

edited

mboersma left a comment

mboersma Sep 8, 2020

jackfrancis Sep 8, 2020

mboersma Sep 8, 2020

mboersma Sep 8, 2020

mboersma Sep 8, 2020

mboersma Sep 8, 2020

mboersma Sep 8, 2020

acs-bot commented Sep 8, 2020

jackfrancis commented Sep 8, 2020

acs-bot commented Sep 9, 2020

jackfrancis commented Sep 10, 2020

chore: reduce customData payload #3793

chore: reduce customData payload #3793

Conversation

jackfrancis commented Sep 4, 2020

codecov bot commented Sep 4, 2020 • edited

Codecov Report

mboersma left a comment

Choose a reason for hiding this comment

mboersma Sep 8, 2020

Choose a reason for hiding this comment

jackfrancis Sep 8, 2020

Choose a reason for hiding this comment

mboersma Sep 8, 2020

Choose a reason for hiding this comment

mboersma Sep 8, 2020

Choose a reason for hiding this comment

mboersma Sep 8, 2020

Choose a reason for hiding this comment

mboersma Sep 8, 2020

Choose a reason for hiding this comment

mboersma Sep 8, 2020

Choose a reason for hiding this comment

acs-bot commented Sep 8, 2020

jackfrancis commented Sep 8, 2020

acs-bot commented Sep 9, 2020

jackfrancis commented Sep 10, 2020

codecov bot commented Sep 4, 2020 •

edited