Use the AzureResource Informer; Refactor for clarity #20

draychev · 2020-01-03T20:46:37Z

Sincere apologies for the large PR. I prefer small, atomic PRs; this is not it.
The goal here is to make one last PR to clean-up the move from a previous repo. Also change the demo so it uses AzureResource CRD.

Changes in this PR

newcomer to the code base is GetComputeIDForService() -- this is a function, which when given a service name, determines where the endpoints forming this service are (what clusters, clouds, compute providers) -- this leverages the new AzureResource CRD and completes the demo using Service -> AzureResource as discussed in Create 'Service' CRD to register and reference k8s and non-k8s compute resources servicemeshinterface/smi-spec#67 (comment)
start the informers for the AzureResource and figure out what Azure URI to query ARM for from Service --> AzureResource match based on labels
No need to pass AZURE_RESOURCE_GROUP around - this is in the AzureResource CRD
gRPC tools moved to pkg/utils
The cmd/eds/clients.go, which had the setupClients function, was simplified and merged into cmd/eds/eds.go
both eds and sds CLI tools get port arg, with the appropriate default (addresses a request by @vramakrishnan in mesh: Adding mesh.provider types enum #16)
added minimal syscall handlers
added sample AzureResource custom k8s resource
moved run-eds.sh and run-sds.sh into ./scripts/ (demo script adjustments if necessary will follow shortly)
added a periodic job running every 5 seconds. This is very short term tool to help w/ development
added a mutex to ServiceCatalog to prepare this for concurrent updates
computeProviders is now a list (no longer a map) -- no need for Compute Provider enum (previous PR closed)
tweaked some of the interfaces -- PLEASE NOTE that Go interfaces are discussed separately here: rfc: Interfaces #22 -- the PR here does NOT intend to improve interfaces.
the Kubernetes compute provider and the MeshSpec are now 2 separate objects (used to be the same implementing 2 interfaces) -- for clarity - easier to comprehend

Interfaces Design Doc

Proper Interfaces design will be done via the following Design Doc (in the shape of a PR to facilitate discussion): #22

draychev · 2020-01-07T20:02:39Z

.env.example

@@ -2,6 +2,5 @@

 export K8S_NAMESPACE=smc
 export AZURE_SUBSCRIPTION=abc
-export AZURE_RESOURCE_GROUP=rg


We no longer need AZURE_RESOURCE_GROUP - the subscription UUID is now available in the AzureResource CRD

draychev · 2020-01-07T20:03:43Z

cmd/eds/clients.go

@@ -1,58 +0,0 @@
-package main


The cmd/eds/clients.go had the setupClients function, which is now simplified and part of cmd/eds/eds.go

draychev · 2020-01-07T20:05:39Z

cmd/sds/sds.go

@@ -34,10 +35,16 @@ func main() {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()

-	grpcServer, lis := cmd.NewGrpc(serverType, port)
+	grpcServer, lis := utils.NewGrpc(serverType, *port)


as @vramakrishnan requested - we moved the gRPC tools in pkg/utils

draychev · 2020-01-07T20:08:52Z

cmd/sds/sds.go

+	sigChan := make(chan os.Signal, 1)
+	signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
+	<-sigChan
+
+	glog.Info("Goodbye!")


The syscall handling block here is a bit of a noop at the moment. Reasons for putting it:
a) symmetry with eds.go
b) we are going to need this once proper integrations w/ a certificate management system(s) is implemented

draychev · 2020-01-07T20:20:39Z

pkg/catalog/types.go

 	servicesCache    map[mesh.ServiceName][]mesh.IP
-	computeProviders map[string]mesh.ComputeProviderI
-	meshSpecProvider mesh.SpecI
+	computeProviders []mesh.ComputeProviderI


The list of ComputeProviders (naming is discussed elsewhere) is now just a list. No need to categorize them in a map.

draychev · 2020-01-07T20:21:53Z

pkg/envoy/eds/server.go

 	glog.Info("[EDS] Create NewEDSServer...")
 	return &EDS{
-		ctx:              ctx,
-		catalog:          catalog,
-		computeProviders: computeProviders,


Removing computeProviders - this should be an implementation detail of the catalog
There is a separate design doc / PR where this is discussed.

draychev · 2020-01-07T20:25:07Z

pkg/mesh/types.go

-	Run(stopCh <-chan struct{}) error
+	// Retrieve the IP addresses comprising the ServiceName.
+	GetIPs(ServiceName) []IP
+	GetID() string


Adding GetID() here -- although this is officially debated in another PR / design doc and will most likely change once the design has settled.

draychev · 2020-01-07T20:53:25Z

pkg/utils/grpc.go

@@ -1,14 +1,13 @@
-package cmd
+package utils


Moving gRPC to the utils pkg

cmd/eds/eds.go

pkg/catalog/catalog.go

draychev · 2020-01-08T21:14:09Z

@snehachhabria thanks for the review!
I addressed the comments you left - could you PTAL!

snehachhabria · 2020-01-08T21:20:39Z

pkg/providers/kube/mesh_spec.go

+type kv struct {
+	k string
+	v string
+}


Should this be renamed as KeyValue struct, key string and values string respectively ?

snehachhabria · 2020-01-08T21:26:05Z

cmd/eds/eds.go

@@ -31,6 +31,9 @@ const (

 	maxAuthRetryCount = 10
 	retryPause        = 10 * time.Second
+
+	azureProvidername      = "Azure"


Nit : casing

snehachhabria

@draychev the changes LGTM

draychev added 16 commits January 3, 2020 15:41

eds: Use the AzureResource Informer

c47406a

kube: Refactor Kubernetes Provider

120253e

mesh: Adding mesh.provider types enum

bc6a11f

azure: Split Client and Provider

9c6d111

k8s: Split Client and Provider

849601a

mesh: No need for ServiceProviderI

7eba42b

Refactor

3d649f8

azure: Adding tests

019c72a

kube: Adding tests

d899eb6

Moving run-*.sh to ./scripts

fab06b1

Remove AZURE_RESOURCE_GROUP

2308aa5

Remove AZURE_RESOURCE_GROUP

4908690

Better AzureResource

cf316cf

Move gRPC tools to pkg/utils/grpc.go

3eb21cf

Port number as a CLI argument

c2104bd

Handle SIGINT / SIGTERM

6fc7e22

draychev added the wip Work-in-Progress label Jan 3, 2020

draychev mentioned this pull request Jan 3, 2020

mesh: Adding mesh.provider types enum #16

Closed

Refactor

62f54fc

draychev force-pushed the draychev/use-azure-resource-4 branch from dfe86e4 to 62f54fc Compare January 3, 2020 21:10

Merge branch 'master' into draychev/use-azure-resource-4

d06e2ce

draychev commented Jan 7, 2020

View reviewed changes

Delete unused Providers enum

633463e

draychev commented Jan 7, 2020

View reviewed changes

mesh.GetComputeIDForService should return a slice of ComputeIDs

efb12ea

draychev commented Jan 7, 2020

View reviewed changes

pkg/utils/grpc.go

@@ -1,14 +1,13 @@

package cmd

package utils

Copy link

Contributor Author

draychev Jan 7, 2020

Choose a reason for hiding this comment

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

Moving gRPC to the utils pkg

draychev added 2 commits January 7, 2020 12:59

Adding comments

7510205

Refactor for clarity; Bring struct declaration close to where it is used

60ce065

draychev removed the wip Work-in-Progress label Jan 7, 2020

draychev requested review from aanandr, asridharan, lachie83, michelleN, snehachhabria and vramakrishnan January 7, 2020 21:24

Merge branch 'master' into draychev/use-azure-resource-4

825ad38

snehachhabria reviewed Jan 8, 2020

View reviewed changes

cmd/eds/eds.go Outdated Show resolved Hide resolved

snehachhabria reviewed Jan 8, 2020

View reviewed changes

cmd/eds/eds.go Show resolved Hide resolved

pkg/catalog/catalog.go Outdated Show resolved Hide resolved

draychev mentioned this pull request Jan 8, 2020

eds: Use the AzureResource Informer #18

Closed

draychev added 3 commits January 8, 2020 13:05

Fix a typo

b55d1ea

Leave a TODO to consolidate parseFlags

f4264c9

Move provider identifiying string to a string const

ed6645b

snehachhabria reviewed Jan 8, 2020

View reviewed changes

snehachhabria self-requested a review January 8, 2020 21:27

snehachhabria approved these changes Jan 8, 2020

View reviewed changes

draychev merged commit 612802a into master Jan 8, 2020

draychev mentioned this pull request Jan 8, 2020

Describe provider identifiying string consts #28

Merged

draychev deleted the draychev/use-azure-resource-4 branch January 15, 2020 23:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the AzureResource Informer; Refactor for clarity #20

Use the AzureResource Informer; Refactor for clarity #20

draychev commented Jan 3, 2020 •

edited

Loading

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev Jan 7, 2020

draychev commented Jan 8, 2020

snehachhabria Jan 8, 2020

snehachhabria Jan 8, 2020

snehachhabria left a comment

Use the AzureResource Informer; Refactor for clarity #20

Use the AzureResource Informer; Refactor for clarity #20

Conversation

draychev commented Jan 3, 2020 • edited Loading

Changes in this PR

Interfaces Design Doc

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

draychev commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snehachhabria left a comment

Choose a reason for hiding this comment

draychev commented Jan 3, 2020 •

edited

Loading