Skip to content

Conversation

@pjohnst5
Copy link
Contributor

@pjohnst5 pjohnst5 commented Jun 17, 2020

What this PR does / why we need it:
First iteration of CNS Request Controller for the AKS on Swift scenario

with an example of how to call the methods in cns/examplerequestcontroller/example/main.go

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #583 into master will increase coverage by 1.83%.
The diff coverage is 32.17%.

@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
+ Coverage   38.05%   39.88%   +1.83%     
==========================================
  Files          41       43       +2     
  Lines        4904     4583     -321     
==========================================
- Hits         1866     1828      -38     
+ Misses       2783     2502     -281     
+ Partials      255      253       -2     

@pjohnst5 pjohnst5 changed the title Request controller reconcile loop Request controller same binary as CNS Jun 17, 2020
@pjohnst5 pjohnst5 marked this pull request as ready for review June 18, 2020 00:59
@matmerr matmerr self-requested a review June 23, 2020 17:12
@pjohnst5 pjohnst5 changed the title Request controller same binary as CNS Request controller for CNS Jun 23, 2020
paulgmiller
paulgmiller previously approved these changes Jun 24, 2020
@matmerr matmerr added the cns Related to CNS. label Jun 25, 2020
Copy link
Collaborator

@thatmattlong thatmattlong left a comment

Choose a reason for hiding this comment

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

Requested some changes. The main one being I think the CNSClient interface and implementation should not be part of the requestcontroller package, and instead should live in cns or a new package cns/client

Copy link
Collaborator

@thatmattlong thatmattlong left a comment

Choose a reason for hiding this comment

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

There is one comment above for a type definition that I don't believe is being used anywhere. However, since that doesn't affect functionality, I'm approving these changes. If you remove that, I will re-approve.

@pjohnst5 pjohnst5 merged commit a15ff46 into Azure:master Jul 2, 2020
@pjohnst5 pjohnst5 deleted the request-controller-reconcile-loop branch July 6, 2020 15:46
pjohnst5 added a commit to pjohnst5/azure-container-networking that referenced this pull request Jul 9, 2020
* NewRequestController and StartRequestController

* Making Start Manager in go routine

* Lookup HOSTNAME env var

* Adding cnsipaminterface.go

* Created requestController interface and implemented updating CRD

* fix windows 1903 test apimodel.json (Azure#585)

* Avoiding redundant calls into cns by only watching for status updates in reconcile loop, ignoring spec updates in reconcil loop. Also adding ability for updating CRD spec through k8sRequestController methods

* fixing comments

* Cleaned up code and added more comments

* Made client interface for testing purposes and changed structure of files to be less folder-y

* Addressed comments from Paul Miller and Wei

* Beginning unit tests

* Finished unit tests

* Fixing pipeline issues

* found issue, fixed HOSTNAME environment variable dependency

* review changes requested

* more review changes

* Addressed changes from yesterday's review

* Changing makefile line to run correct package

* Addressed Matt Long's suggestions

Co-authored-by: Mathew Merrick <matmerr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cns Related to CNS.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants