Add dependencyproxy model, crds, listers, informers #279
Conversation
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #279 +/- ##
==========================================
- Coverage 74.18% 73.06% -1.12%
==========================================
Files 32 33 +1
Lines 3266 3316 +50
==========================================
Hits 2423 2423
- Misses 695 745 +50
Partials 148 148
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
aattuluri
changed the title
|
|
||
| message Proxy { | ||
| // Identifier of the proxy's workload | ||
| string identity = 3; |
There was a problem hiding this comment.
Just to confirm, admiral.io/env label on the dependency proxy would be used in addition to this identity to construct the proxy endpoint?
There was a problem hiding this comment.
yes, that is correct. Let me know if you want to make it more explicit to avoid confusion.
There was a problem hiding this comment.
I think we can add an example in the docs with a sample dependencyproxy and what Admiral renders
| kind: DependencyProxy | ||
| listKind: DependencyProxyList | ||
| plural: dependencyproxies | ||
| singular: dependencyproxy |
There was a problem hiding this comment.
Can we add a shortName for the CRD, something like dp?
There was a problem hiding this comment.
yes, will do that
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
| // spec: | ||
| // hosts: | ||
| // - test0.stage.greeting.xyz | ||
| // - test1.stage.greeting.xyz |
There was a problem hiding this comment.
I am thinking if these should be test0.greeting.xyz and test1.greeting.xyz because if some one has e2e1 and e2e2, the endpoints will look like e2e1.e2e.greeting.xyz and e2e2.e2e.greeting.xyz. Also this will make the migration later (from gateway to mesh seamless when that happens)
There was a problem hiding this comment.
hey @aattuluri , I'm sorry this example I think is a little confusing. stage here is the admiral.io/env on the deployment/rollout of the destination service(greeting). So I was thinking if we'll prepend the prefixes defined above to the generated endpoint of the service. `..
So we'll be generating the regular endpoint without the prefix as well as shown here
This should make the migration seamless. Please correct me if I'm wrong.
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
admiral/pkg/apis/admiral/v1/type.go
Outdated
|
|
||
| // DependencyProxyStatus is the status for a DependencyProxy resource | ||
| type DependencyProxyStatus struct { | ||
| ClusterSynced int32 `json:"clustersSynced"` |
There was a problem hiding this comment.
What is the logic to update this?
There was a problem hiding this comment.
@nirvanagit , since this was in the other types, so I just kept it as is.
@aattuluri , do you know if we need this or if this can be removed?
There was a problem hiding this comment.
@shriramsharma We should remove this given its doesnt make sense for this CRD.
| ) | ||
|
|
||
| // DepProxyHandler interface contains the methods that are required | ||
| type DepProxyHandler interface { |
There was a problem hiding this comment.
DependencyProxyHandler?
There was a problem hiding this comment.
Is the struct which implements this already in this PR?
| informer cache.SharedIndexInformer | ||
| } | ||
|
|
||
| type depProxyCache struct { |
|
|
||
| func NewDependencyProxyController(stopCh <-chan struct{}, handler DepProxyHandler, configPath string, namespace string, resyncPeriod time.Duration) (*DependencyProxyController, error) { | ||
|
|
||
| depProxyController := DependencyProxyController{} |
There was a problem hiding this comment.
may be just controller?
| return nil, fmt.Errorf("failed to create dependency controller k8s client: %v", err) | ||
| } | ||
|
|
||
| depProxyController.DepCrdClient, err = AdmiralCrdClientFromPath(configPath) |
There was a problem hiding this comment.
Should we call it admiralCRDClient? I see its called DepCrdClient even for dependency.
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
fixes #278