charts/osm: use image digest for default images #4108
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| name: Pre-release | ||
| on: | ||
| push: | ||
| tags: | ||
| - "pre-rel-v*" | ||
|
|
||
| jobs: | ||
| version: | ||
| name: Set Version from git ref | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| version: ${{ steps.version.outputs.version }} | ||
| steps: | ||
| - id: version | ||
| run: echo "::set-output name=version::$(sed 's#^refs/tags/pre-rel-\(.*\)#\1#' <<< '${{ github.ref }}')" | ||
|
|
||
| images: | ||
| name: Docker Images | ||
| runs-on: ubuntu-latest | ||
| needs: version | ||
| env: | ||
| DOCKER_USER: ${{ secrets.RELEASE_DOCKER_USER }} | ||
| DOCKER_PASS: ${{ secrets.RELEASE_DOCKER_PASS }} | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v2 | ||
| - name: Restore Module Cache | ||
| uses: actions/cache@v2 | ||
| with: | ||
| path: ~/go/pkg/mod | ||
| key: ${{ runner.os }}-gomod2-${{ hashFiles('**/go.sum') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gomod2- | ||
| - name: Restore Build Cache | ||
| uses: actions/cache@v2 | ||
| with: | ||
| path: ~/.cache/go-build | ||
| key: ${{ runner.os }}-gobuild-${{ hashFiles('**/*.go') }} | ||
| - name: Setup Go 1.16 | ||
| uses: actions/setup-go@v1 | ||
| with: | ||
| go-version: 1.16 | ||
| - name: Docker Login | ||
| run: docker login --username "$DOCKER_USER" --password-stdin <<< "$DOCKER_PASS" | ||
| - name: Push images with version tag | ||
| env: | ||
| CTR_TAG: ${{ needs.version.outputs.version }} | ||
| run: make docker-push VERIFY_TAGS=1 | ||
| - name: Push images with latest tag | ||
| env: | ||
| CTR_TAG: latest | ||
| run: make docker-push | ||
| - name: Upload image digest | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: osm_image_digests | ||
| path: /tmp/osm_image_digest_* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,40 +105,3 @@ jobs: | |
| asset_path: _dist/sha256sums.txt | ||
| asset_name: sha256sums.txt | ||
| asset_content_type: text/plain | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,12 +7,25 @@ OpenServiceMesh: | |
| # | ||
| # -- OSM control plane image parameters | ||
| image: | ||
| # -- Container image registry for control plane images | ||
| registry: openservicemesh | ||
| # -- Container image pull policy for control plane containers | ||
| pullPolicy: IfNotPresent | ||
| # -- Container image tag for control plane images | ||
| tag: "latest-main" | ||
|
Comment on lines
+12
to
+15
There was a problem hiding this comment. Let's change the pull policy to There was a problem hiding this comment. I didn't change this just because the demo scripts sets this to Always. We have to be careful in not setting this to Always for release versions, so it makes sense to default it to There was a problem hiding this comment. @nojnhuh, what's the reason behind your request to change to There was a problem hiding this comment.
I suppose we could effectively get this behavior "for free" if the
Isn't that exactly the distinction we're making here with the tags, where the main branch is set up for development and the release branches are set up for production? There was a problem hiding this comment. @nojnhuh I don't mind changing the PullPolicy to |
||
| # -- Image digest (defaults to latest compatible tag) | ||
| digest: | ||
| # -- osm-controller's image digest | ||
| osmController: "" | ||
| # -- osm-injector's image digest | ||
| osmInjector: "" | ||
| # -- Sidecar init container's image digest | ||
| osmSidecarInit: "" | ||
| # -- osm-crds' image digest | ||
| osmCRDs: "" | ||
| # -- osm-boostrap's image digest | ||
| osmBootstrap: "" | ||
|
|
||
|
|
||
| # -- `osm-controller` image pull secret | ||
| imagePullSecrets: [] | ||
|
|
||
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.
For readability - I wonder if instead of
OpenServiceMesh.image.digest.osmBootstrapthis should beOpenServiceMesh.image.osmBootstrap.digest(i.e. swap the positions ofdigestandosmBootstrap)This mainly because I see that the comment to this value is
osm-boostrap's image digest(first name of image, then digest)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.
That works, I felt it's easier to keep
image.digestas the top-level key and use the image names as child keys, so we have:instead of:
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'm not sure it makes much of a difference now, but if we decide later to encode more things for each image, like if we make it possible to pull different images from different repos and with different tags, then it definitely makes sense to reorganize the values to
image.<name>.digest. But reorganizing the values like that would be a breaking change to the Helm chart which we should do our best to avoid.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.
For simplicity, I don't think we should be allowing mixing tags for different images, that's an unnecessary complexity I wouldn't consider at the moment. We have been using the image tags property as before, and given that only the digests are different it makes sense to keep this change as is, else it would also make sense to move the tags, pullPolicy etc. per image which I think we should avoid.