resource deployment #5
Conversation
| const grpcServiceName = 'collector-grpc' | ||
| const grpcPort = 4317 | ||
| const udpServiceName = 'collector-udp' | ||
| const udpPort = 55690 | ||
| const tcpServiceName = 'collector-tcp' | ||
| const httpPort = 4318 |
There was a problem hiding this comment.
If these are fixed values do they belong in this scope? Could we instead move them down a level, this would prevent us from having to pass them in as props to the to the sample app construct.
There was a problem hiding this comment.
Since the collector needs these values we can just export them from the construct and reference them that way.
There was a problem hiding this comment.
Or we could pass in the sample app as a prop to the collector and allow the collector construct to handle those values.
There was a problem hiding this comment.
I wasn't sure. On the one hand that is a lot cleaner, but if we're looking at the sample app from a more zoomed out perspective, would we want to allow users to configure the sample app the way that they want? Maybe we can have default values with optional parameters that the user can use to override them?
| const listenAddressHost = '0.0.0.0' | ||
| const listenAddressPort = 8080 |
There was a problem hiding this comment.
Same comment as above, should we move these into the sample app construct as fixed values?
| }) | ||
|
|
||
| // used for when sample app is push mode | ||
| const deployServices = props.sampleAppMode === 'push' |
There was a problem hiding this comment.
Isn't a service required for both push and pull sample apps?
There was a problem hiding this comment.
Those are services for the collector. I believe those are only required for a push mode sample app where the collector receives the data from the sample app. For the pull mode case, the collector goes and scrapes the data from the sample app (through the sample app service) so it doesn't need its own services. I believe the TF framework also had this condition in the service deployments.
| cluster: props.cluster, | ||
| namespaceConstruct: props.namespaceConstruct, | ||
| sampleAppLabel: props.sampleAppLabel, | ||
| sampleAppImageURI: props.sampleAppImageURI, | ||
| grpcServiceName: props.grpcServiceName, | ||
| grpcPort: props.grpcPort, | ||
| udpServiceName: props.udpServiceName, | ||
| udpPort: props.udpPort, | ||
| tcpServiceName: props.tcpServiceName, | ||
| httpPort: props.httpPort, | ||
| listenAddressHost: props.listenAddressHost, | ||
| listenAddressPort: props.listenAddressPort, | ||
| region: props.region | ||
| }) | ||
| } | ||
| else if (props.sampleAppMode === 'pull'){ | ||
| new PullModeSampleAppDeploymentConstruct(this, 'pull-mode-sample-app-construct', { | ||
| cluster: props.cluster, | ||
| namespaceConstruct: props.namespaceConstruct, | ||
| sampleAppLabel: props.sampleAppLabel, | ||
| sampleAppImageURI: props.sampleAppImageURI, | ||
| listenAddressHost: props.listenAddressHost, | ||
| listenAddressPort: props.listenAddressPort, | ||
| region: props.region |
There was a problem hiding this comment.
Can we use interfaces to expose these props and then do something like <push/pull>sampleAppProps = props as <push/pull>SampleAppProps?
There was a problem hiding this comment.
We could export the props interface in the push and pull mode sample apps and instantiate an instance of that interface, but then the same code would be required to instantiate the appropriate interface and give it the required props. Maybe I'm misunderstanding the suggestion.
Description:
Added CDK construct classes to represent Kubernetes resources as well as code to deploy these constructs based on user-provided configuration for a test-case.
To outline the general control flow: The
deploy_resourcesfunction inresource-deployment.tsparses the configuration for the test-case (as provided in a YAML file) and instantiates a TestCaseResourceDeploymentConstruct passing the data as props. In turn, this construct instantiates a low-levelNamespaceConstructand high-level constructsGeneralSampleAppDeploymentConstructandGeneralCollectorDeploymentConstruct, passing them the necessary props. Finally, based on the props these constructs instantiate the appropriate low-level constructs that define manifests for Kubernetes resources.Documentation:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.