test: E2E fix scale test indexing out of bounds

[JesusAlvarezTorres]

Reason for Change:

Fixing a test issue on E2E scale tests, the loop iterating the node pools needs to stop when the index is less than the total of agent pool profiles; currently the fix causes an additional call to scale with node poll set to null

• (( ++i ))

• (( i <= 1 ))

++ jq -r --arg i 1 '. | .properties.agentPoolProfiles[$i | tonumber].name'

• nodepool=null

• '[' '' = true ']'

Related to changes made in this PR: #3830

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

• No
• Yes

If "Yes," did you notify that project's maintainers and provide attribution?

• No
• Yes

Requirements:

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

Notes:

[JesusAlvarezTorres]

@jackfrancis can you please take a look at this change? It is a small test fix to some recent changes to cluster.sh

[mboersma]

/azp run pr-e2e

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[codecov]

Codecov Report

Merging #3873 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3873   +/-   ##
=======================================
  Coverage   72.83%   72.83%           
=======================================
  Files         149      149           
  Lines       23171    23173    +2     
=======================================
+ Hits        16876    16878    +2     
  Misses       5178     5178           
  Partials     1117     1117           

Impacted Files	Coverage Δ
cmd/scale.go	12.63% <ø> (ø)
cmd/addpool.go	18.07% <100.00%> (ø)
cmd/deploy.go	60.59% <100.00%> (ø)
pkg/api/vlabs/validate.go	78.92% <100.00%> (+0.03%)	⬆️

---

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 1539189...fd871d9. Read the comment docs.

[jackfrancis]

Hmmm, I am seeing the opposite problem:

++ jq -r '.properties.agentPoolProfiles[].name'
 + nodepools='poollinux
 poolwin
 pooladded'
 + (( i = 0 ))
 + (( i <= 1 ))

In the above the nodepools "array" has 3 items, but ${#nodepools[@]} is evaluating to the number 1.

I think we're not correctly using bash to evaluate the number of pools in the string array returned from $(jq -r '.properties.agentPoolProfiles[].name' < _output/$RESOURCE_GROUP/apimodel.json)

[JesusAlvarezTorres]

Hmmm, I am seeing the opposite problem:

++ jq -r '.properties.agentPoolProfiles[].name'
 + nodepools='poollinux
 poolwin
 pooladded'
 + (( i = 0 ))
 + (( i <= 1 ))

In the above the nodepools "array" has 3 items, but ${#nodepools[@]} is evaluating to the number 1.

I think we're not correctly using bash to evaluate the number of pools in the string array returned from $(jq -r '.properties.agentPoolProfiles[].name' < _output/$RESOURCE_GROUP/apimodel.json)

@jackfrancis
I just did a quick test by changing the loop to something like this:

# Hard coded API model path for testing the loop
nodepools=$(jq -r '.properties.agentPoolProfiles[].name' < /mnt/c/tmp/_apimodel.json)
for i in "${nodepools[@]}"; do
nodepool=$i
echo "$nodepool"
done

In that case $i gives the name of the node pool, so there is no need to do the second jq query to translate from numerical index to node pool name... do we need the numerical index as well or would this approach work?

[JesusAlvarezTorres]

Hmmm, I am seeing the opposite problem:

++ jq -r '.properties.agentPoolProfiles[].name'
 + nodepools='poollinux
 poolwin
 pooladded'
 + (( i = 0 ))
 + (( i <= 1 ))

In the above the nodepools "array" has 3 items, but ${#nodepools[@]} is evaluating to the number 1.
I think we're not correctly using bash to evaluate the number of pools in the string array returned from $(jq -r '.properties.agentPoolProfiles[].name' < _output/$RESOURCE_GROUP/apimodel.json)

@jackfrancis
I just did a quick test by changing the loop to something like this:

# Hard coded API model path for testing the loop
nodepools=$(jq -r '.properties.agentPoolProfiles[].name' < /mnt/c/tmp/_apimodel.json)
for i in "${nodepools[@]}"; do
nodepool=$i
echo "$nodepool"
done

In that case $i gives the name of the node pool, so there is no need to do the second jq query to translate from numerical index to node pool name... do we need the numerical index as well or would this approach work?

My comment above didn't work, it was as you said @jackfrancis. There was something wrong in how the node pool number was being thrown by the jq query. I updated that part in a new commit. Please take a look.

[JesusAlvarezTorres]

/assign @jackfrancis

[jackfrancis]

Thanks! Testing your change now

[jackfrancis]

/lgtm

[jackfrancis]

Tested out O.K., thanks for fixing this @JesusAlvarezTorres!

[acs-bot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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]

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