Skip to content

Conversation

@alfrunes
Copy link
Contributor

No description provided.

@mender-test-bot
Copy link

There was an error running your pipeline, see logs for details.

@alfrunes alfrunes force-pushed the ALV-49-take3 branch 3 times, most recently from 0f9e6a4 to 7c570b6 Compare September 14, 2023 12:33
@mender-test-bot
Copy link

There was an error running your pipeline, see logs for details.

@mender-test-bot
Copy link

There was an error running your pipeline, see logs for details.

@alfrunes alfrunes force-pushed the ALV-49-take3 branch 2 times, most recently from 9fc8b47 to 20c5a85 Compare September 14, 2023 12:37
@mender-test-bot
Copy link

There was an error running your pipeline, see logs for details.

Copy link
Contributor

@oldgiova oldgiova left a comment

Choose a reason for hiding this comment

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

LGTM, just minor suggestions

- if: '$CI_COMMIT_REF_NAME =~ /^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z-]+)?/'
when: manual
- when: never
image: "quay.io/skopeo/stable:${SKOPEO_VERSION}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to me. Minor (optional) suggestion: switch skopeo for regctl, just to have a unique standard tool. reference:

  image: "registry.gitlab.com/northern.tech/mender/mender-test-containers:docker-multiplatform-buildx-v1-master"

regctl image copy ${GITLAB_REGISTRY_TAG} $DOCKER_REPOSITORY:$DOCKER_PUBLISH_TAG

But up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer using as few in-house images as possible in our pipelines since this generally leads to more maintenance overhead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer using as few in-house images as possible in our pipelines since this generally leads to more maintenance overhead.

off-house images have given us headaches in the past too 😅 There it no golden rule.

build:docker:
rules:
- when: never
script: ["true"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this optional with the when: never rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that first, but then gitlab complained about the script section being empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically here we should be overriding jobs from the template which have their script. If GitLab complained about script section being empty it means that the job was not added by the template in the first place, and hence no need for the rules to remove it.

You can use the pipeline editor while working on CI to quickly visualize and sanitize the pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... but it would've been added if the branch was staging.

Copy link
Contributor

@lluiscampos lluiscampos left a comment

Choose a reason for hiding this comment

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

Careful with the GOLANG_VERSION ⚠️ That we have to fix. The rest are complements or questions 👍

.gitlab-ci.yml Outdated
Comment on lines 10 to 12
GOLANG_VERSION:
description: "The version of the image: `docker.io/library/golang` to use for testing"
value: "1.21.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Careful about this. This exact variable is parsed by the deb package build system to use exact this version for building.

It is a long story (here and here), but please use a different name.


DOCKER_REGISTRY: "docker.io"
DOCKER_REPOSITORY: northerntechhq/nt-connect

Copy link
Contributor

Choose a reason for hiding this comment

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

Real nice setting the variables ✨

build:docker:
rules:
- when: never
script: ["true"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Basically here we should be overriding jobs from the template which have their script. If GitLab complained about script section being empty it means that the job was not added by the template in the first place, and hence no need for the rules to remove it.

You can use the pipeline editor while working on CI to quickly visualize and sanitize the pipeline.

- if: '$CI_COMMIT_REF_NAME =~ /^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z-]+)?/'
when: manual
- when: never
image: "quay.io/skopeo/stable:${SKOPEO_VERSION}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer using as few in-house images as possible in our pipelines since this generally leads to more maintenance overhead.

off-house images have given us headaches in the past too 😅 There it no golden rule.

"docker://${DOCKER_REPOSITORY}:${CI_COMMIT_SHA}" \
"docker://${DOCKER_REPOSITORY}:${CI_COMMIT_REF_NAME}"
fi
artifacts: {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain me why we need this job in the first place? What are the key differences from the one in the template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses a different set of rules for tagging. We use golang semantic versions for releases vX.Y.Z which results in tag X.Y.Z and X.Y. We don't use saas-* tags. We publish main and staging along with the commit sha.

Signed-off-by: Alf-Rune Siqveland <alf.rune@northern.tech>
Signed-off-by: Alf-Rune Siqveland <alf.rune@northern.tech>
@alfrunes alfrunes merged commit 3b4ac03 into NorthernTechHQ:main Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants