-
Notifications
You must be signed in to change notification settings - Fork 525
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
automate the ironbank generation #8537
Conversation
This pull request does not have a backport label. Could you fix it @v1v? 🙏
NOTE: |
/package |
🌐 Coverage report
|
@v1v The implementation looks pretty sane. EDIT: |
- filename: "apm-server-{{ beat_version }}-linux-x86_64.tar.gz" | ||
url: "<artifact_path>/apm-server-{{ beat_version }}-linux-x86_64.tar.gz" | ||
validation: | ||
type: "sha512" | ||
value: "<insert SHA 512 here>" |
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.
This is the agreement with the Unified Release automation.
📚 Go benchmark reportDiff with the
report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat |
.gitignore
Outdated
@@ -53,3 +53,6 @@ terraform.tfstate.backup | |||
terraform.tfvars | |||
testing/benchmark/.envrc | |||
testing/benchmark/benchmark-result.txt | |||
|
|||
# Ironbank | |||
apm-server-ironbank* |
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.
As long as the Tar
compress the given folder and nested folders, then it's required to run tar
within the top-level folder so the folder name contains the docker context, otherwise it will be not compliance with the Ironbank automation
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.
Sorry, having trouble understanding this. What is the requirement? I'm not keen on build files being created in the top level directory. Why can't the apm-server-ironbank-<version>-docker-build-context
directory be created under build/
?
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.
I tried initially that approach but the generated apm-server-ironbank-<version>-docker-build-context.tar.gz
included the build/...
folders. Unfortunately I could not find the way to chdir
to build
so the tar
command could be created from that context instead.
I might need to look for how to solve this, though if you have some suggestions please let me know.
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.
diff --git a/magefile.go b/magefile.go
index e314dfb5b..7cd8d9d4d 100644
--- a/magefile.go
+++ b/magefile.go
@@ -261,7 +261,8 @@ func saveIronbank() error {
fmt.Println(">> saveIronbank: save the IronBank container context.")
ironbank := getIronbankContextName()
- if _, err := os.Stat(ironbank); os.IsNotExist(err) {
+ buildDir := filepath.Join("build", ironbank)
+ if _, err := os.Stat(buildDir); os.IsNotExist(err) {
return fmt.Errorf("cannot find the folder with the ironbank context")
}
@@ -276,7 +277,7 @@ func saveIronbank() error {
tarGzFile := filepath.Join(distributionsDir, ironbank+".tar.gz")
// Save the build context as tar.gz artifact
- err := mage.Tar(ironbank, tarGzFile)
+ err := mage.Tar(buildDir, tarGzFile)
if err != nil {
return fmt.Errorf("cannot compress the tar.gz file")
}
@@ -297,6 +298,7 @@ func getIronbankContextName() string {
func prepareIronbankBuild() error {
fmt.Println(">> prepareIronbankBuild: prepare the IronBank container context.")
ironbank := getIronbankContextName()
+ buildDir := filepath.Join("build", ironbank)
templatesDir := filepath.Join("packaging", "ironbank")
data := map[string]interface{}{
@@ -306,7 +308,7 @@ func prepareIronbankBuild() error {
err := filepath.Walk(templatesDir, func(path string, info os.FileInfo, _ error) error {
if !info.IsDir() {
target := strings.TrimSuffix(
- filepath.Join(ironbank, filepath.Base(path)),
+ filepath.Join(buildDir, filepath.Base(path)),
".tmpl",
)
but it produces:
$ ls -ltra build
total 0
drwxr-xr-x 6 vmartinez staff 192 Jul 18 14:08 apm-server-ironbank-8.4.0-docker-build-context
drwxr-xr-x 72 vmartinez staff 2304 Jul 18 19:04 ..
drwxr-x--- 5 vmartinez staff 160 Jul 18 19:04 distributions
drwxr-x--- 4 vmartinez staff 128 Jul 18 19:04 .
$ ls -ltra build/distributions
total 24
-rw-r--r-- 1 vmartinez staff 8872 Jul 18 14:08 apm-server-ironbank-8.4.0-docker-build-context.tar.gz
-rw-r--r-- 1 vmartinez staff 183 Jul 18 14:08 apm-server-ironbank-8.4.0-docker-build-context.tar.gz.sha512
$ cd build/distributions
$ tar xvzf apm-server-ironbank-8.4.0-docker-build-context.tar.gz
x build/apm-server-ironbank-8.4.0-docker-build-context
x build/apm-server-ironbank-8.4.0-docker-build-context/Dockerfile
x build/apm-server-ironbank-8.4.0-docker-build-context/LICENSE
x build/apm-server-ironbank-8.4.0-docker-build-context/README.md
x build/apm-server-ironbank-8.4.0-docker-build-context/hardening_manifest.yaml
But I'd expect:
$ tar xvzf apm-server-ironbank-8.4.0-docker-build-context.tar.gz
x apm-server-ironbank-8.4.0-docker-build-context
x apm-server-ironbank-8.4.0-docker-build-context/Dockerfile
x apm-server-ironbank-8.4.0-docker-build-context/LICENSE
x apm-server-ironbank-8.4.0-docker-build-context/README.md
x apm-server-ironbank-8.4.0-docker-build-context/hardening_manifest.yaml
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.
@v1v should it even have apm-server-ironbank-8.4.0-docker-build-context/
in the contents? This is what the 8.3.2 Elasticsearch IronBank build context tarball looks like:
$ tar tf elasticsearch-ironbank-8.3.2-docker-build-context.tar.gz
scripts/
scripts/docker-openjdk
scripts/docker-entrypoint.sh
scripts/elasticsearch.yml
README.md
LICENSE
hardening_manifest.yaml
Dockerfile
scripts/log4j2.properties
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.
Good point, I thought I did review the folder layout for some other projects and it seems I messed up with the extra folder :/
tar -xvf kibana-ironbank-8.3.3-SNAPSHOT-docker-build-context.tar.gz
x Dockerfile
x LICENSE
x README.md
x bin/
x bin/kibana-docker
x config/
x config/kibana.yml
x hardening_manifest.yaml
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.
I'd say mage.Tar
cannot be used as is, I'll transition this PR to draft so I'll make the changes to support the tar.gz
without any nested folders
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.
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.
I don't know much about the ironbark generation, just a generic review on the errors returned
@marclop Could you please give this one last look? Thanks. |
/test |
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.
LGTM. As mentioned on Slack, I'm thinking about introducing our own Dockerfile, so I might end up making some changes. If we do, I'll ping you to find out how we can verify the changes.
Co-authored-by: Andrew Wilkins <[email protected]>
(cherry picked from commit 682a0e5) # Conflicts: # Makefile # magefile.go
(cherry picked from commit 682a0e5) # Conflicts: # Makefile # magefile.go
Motivation/summary
Automate the docker context generation for the IronBank releases, this will allow us to move away from creating those docker context manually in a different repository, since the Platform Release team already provide the automation.
Implementation details
A similar approach was done for Kibana and Elasticsearch
I initially enabled to build and save the docker image but it failed with some auth issues:
I don't know if other teams are actually building those docker.
DoD build context content
As required the build context contains the following files:
Dockerfile
with specific requirements for DoD (registry args, no internet dependencies, healthcheck, ...)hardening_manifest.yaml
with all required dependenciesLICENSE
specific for DoDREADME.md
with specific content for DoDconfig/
(optional) directory containing all config files to include into the Docker imagescripts/
directory containing all scripts to include into the Docker image (example: entrypoint...)Tasks
x86_64
.Test
This job produces the artifacts.
Further details
This could be potentially added to the Beats project, but I don't know what's the plan for the project and what's the priorities for the Platform Productivity team, but for now I think we could try to use this in the
apm-server
repo