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

store Azure CNI version in api model #3804

Merged
merged 1 commit into from
Sep 6, 2018

Conversation

jackfrancis
Copy link
Member

@jackfrancis jackfrancis commented Sep 5, 2018

What this PR does / why we need it: Stores the statically assigned Azure CNI version in the api model so that it is more easily determined which version of Azure CNI a cluster was built with.

Note: this does not enable configurable versioning of Azure CNI at deployment time.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

store Azure CNI version in api model

@acs-bot
Copy link

acs-bot commented Sep 5, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis

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:

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

@ghost ghost assigned jackfrancis Sep 5, 2018
@ghost ghost added the in progress label Sep 5, 2018
@acs-bot acs-bot added the size/M label Sep 5, 2018
@jackfrancis
Copy link
Member Author

@weinong @sharmasushant FYI

@CecileRobertMichon
Copy link
Contributor

CecileRobertMichon commented Sep 5, 2018

What about making it a variable in the ARM template? Only reason I'm saying that is it might be confusing for users to have a field in the apimodel / in their cluster configuration that gets ignored. From what I understand we're not allowing this value to be configurable for now.

@jackfrancis
Copy link
Member Author

@CecileRobertMichon I'm open to rationalizing this whole thing (including making the version configurable) and reflecting that in the ARM template. For now this incremental change is useful.

@codecov
Copy link

codecov bot commented Sep 5, 2018

Codecov Report

Merging #3804 into master will increase coverage by 0.05%.
The diff coverage is 85.71%.

@@            Coverage Diff            @@
##           master   #3804      +/-   ##
=========================================
+ Coverage   55.65%   55.7%   +0.05%     
=========================================
  Files         109     109              
  Lines       16233   16240       +7     
=========================================
+ Hits         9034    9047      +13     
+ Misses       6410    6402       -8     
- Partials      789     791       +2

@jackfrancis jackfrancis merged commit cff5e74 into Azure:master Sep 6, 2018
@ghost ghost removed the in progress label Sep 6, 2018
@jackfrancis jackfrancis deleted the cni-version-api-model branch September 6, 2018 00:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants