-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use Pull through cache #763
base: main
Are you sure you want to change the base?
Changes from all commits
34272c3
e462c07
b2fbdac
7296dbe
b056535
aa0aa65
0585474
6995fc2
d414c64
bbc1c49
d6f794d
b35bd0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |
"fmt" | ||
|
||
"github.com/DataDog/test-infra-definitions/common/utils" | ||
"github.com/DataDog/test-infra-definitions/components/command" | ||
"github.com/DataDog/test-infra-definitions/components/datadog/agent" | ||
"github.com/DataDog/test-infra-definitions/components/datadog/apps/cpustress" | ||
"github.com/DataDog/test-infra-definitions/components/datadog/apps/dogstatsd" | ||
|
@@ -14,12 +15,16 @@ import ( | |
"github.com/DataDog/test-infra-definitions/components/datadog/apps/tracegen" | ||
dogstatsdstandalone "github.com/DataDog/test-infra-definitions/components/datadog/dogstatsd-standalone" | ||
fakeintakeComp "github.com/DataDog/test-infra-definitions/components/datadog/fakeintake" | ||
"github.com/DataDog/test-infra-definitions/components/docker" | ||
localKubernetes "github.com/DataDog/test-infra-definitions/components/kubernetes" | ||
"github.com/DataDog/test-infra-definitions/components/os" | ||
"github.com/DataDog/test-infra-definitions/components/remote" | ||
resAws "github.com/DataDog/test-infra-definitions/resources/aws" | ||
"github.com/DataDog/test-infra-definitions/scenarios/aws/ec2" | ||
"github.com/DataDog/test-infra-definitions/scenarios/aws/fakeintake" | ||
|
||
goremote "github.com/pulumi/pulumi-command/sdk/go/command/remote" | ||
|
||
"github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes" | ||
"github.com/pulumi/pulumi/sdk/v3/go/pulumi" | ||
) | ||
|
@@ -39,9 +44,20 @@ func Run(ctx *pulumi.Context) error { | |
return err | ||
} | ||
|
||
// Install docker if not installed yet, we need it to configure docker credentials | ||
_, dockerInstallCmd, err := docker.NewManager(*awsEnv.CommonEnvironment, vm) | ||
if err != nil { | ||
return err | ||
} | ||
// Configure ECR credentials for use in Kind | ||
ecrLoginCommand, err := ConfigureECRCredentials(awsEnv, vm, osDesc.Architecture, utils.PulumiDependsOn(dockerInstallCmd)) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
kindClusterName := ctx.Stack() | ||
|
||
kindCluster, err := localKubernetes.NewKindCluster(*awsEnv.CommonEnvironment, vm, awsEnv.CommonNamer.ResourceName("kind"), kindClusterName, awsEnv.KubernetesVersion()) | ||
kindCluster, err := localKubernetes.NewKindCluster(*awsEnv.CommonEnvironment, vm, awsEnv.CommonNamer.ResourceName("kind"), kindClusterName, awsEnv.KubernetesVersion(), utils.PulumiDependsOn(ecrLoginCommand)) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -154,3 +170,36 @@ agents: | |
|
||
return nil | ||
} | ||
|
||
func ConfigureECRCredentials(e resAws.Environment, vm *remote.Host, arch os.Architecture, opts ...pulumi.ResourceOption) (*goremote.Command, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 suggestion There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since it is something specific to AWS I did not want to have it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||
architecture := "x86_64" | ||
if arch == os.ARM64Arch { | ||
architecture = "aarch64" | ||
} | ||
|
||
unzipInstallCommand, err := vm.OS.PackageManager().Ensure("unzip", nil, "", opts...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 thought There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think replacing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure there is an added value in pinning versions, for two reasons:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do pin some AMIs, for example for the installer tests. We don't probably want to maintain all images and versions, but for folks who would like to, we might want to offer the option. No one ever asked for it, just noticing it now. |
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
awsCliInstallCommand, err := vm.OS.Runner().Command( | ||
e.CommonNamer.ResourceName("aws-cli-install"), | ||
&command.Args{ | ||
Create: pulumi.Sprintf("command -v aws || curl 'https://awscli.amazonaws.com/awscli-exe-linux-%s.zip' -o 'awscliv2.zip' && unzip awscliv2.zip && sudo ./aws/install", architecture), | ||
}, | ||
utils.PulumiDependsOn(unzipInstallCommand), | ||
) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
ecrLoginCommand, err := vm.OS.Runner().Command( | ||
e.CommonNamer.ResourceName("ecr-login"), | ||
&command.Args{ | ||
Create: pulumi.Sprintf("aws ecr get-login-password | docker --config /tmp/kind-config login --username AWS --password-stdin %s", e.CloudProviderEnvironment.InternalRegistry()), | ||
}, | ||
utils.PulumiDependsOn(awsCliInstallCommand), | ||
) | ||
pducolin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return ecrLoginCommand, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need docker installed before running
ConfigureECRCredentials
otherwisedocker login
fails.This is not ideal because the
NewKindCluster
already installsdocker
but it works because docker installation is idempotent.We could do the installation of docker oustide of
NewKindCluster
or add ahook
to configure the credentials after docker is installed. The latter option would be easier if all theEnvironment
to implement a common interface (#688)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine, I am wondering if we should have it inside the
NewKindCluster
component, as it needs it. Can be done laterThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would have been easier, but we do not want to put AWS specific logic in the
NewKindCluster
component. That could be used on any cloud providerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to duplicate this code in
datadog-agent
with current code. However you could have aWithECRCredentials
that implements this. Note theExtraMount
you're adding means that it will currently fail when not using any credential helper. You may need to always create a empty{}
JSON file.You also don't need
docker login
, whatdocker login
does is very basic JSON gen. The way I see it the clean way would be to build a smallGo
code that imports https://github.com/awslabs/amazon-ecr-credential-helper/blob/main/ecr-login/ecr.go#L83-L110 and produces theconfig.json
onstdout
.It's much smaller and faster to download then the AWS SDK and you have the option to package as a docker container and get the
config.json
withdocker run --network host <your_helper_image> > config-tmp.json
.