Skip to content
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

Fix MC Docker tags #1539

Merged
merged 3 commits into from
Feb 14, 2025
Merged

Conversation

JackPGreen
Copy link
Contributor

Update MC Docker tags to hazelcast/management-center:{page-latest-supported-mc}.

Updated from:

  • latest-snapshot - should use released artefacts
  • full-version - the versions of Hazelcast platform and MC are not tied (e.g. no 5.5.2 MC)

Update MC Docker tags to `hazelcast/management-center:{page-latest-supported-mc}`.

Updated from:
- `latest-snapshot` - should use released artefacts
- `full-version` - the versions of Hazelcast platform and MC are not tied (e.g. no `5.5.2` MC)
@JackPGreen JackPGreen added the backport to all versions Backport commits to maintenance branches (from main) label Feb 12, 2025
@JackPGreen JackPGreen self-assigned this Feb 12, 2025
@JackPGreen JackPGreen requested a review from a team as a code owner February 12, 2025 22:17
Copy link

netlify bot commented Feb 12, 2025

👷 Deploy Preview for hardcore-allen-f5257d processing.

Name Link
🔨 Latest commit 4f8ee17
🔍 Latest deploy log https://app.netlify.com/sites/hardcore-allen-f5257d/deploys/67ad1e115bf19d0008eb0038

Copy link

netlify bot commented Feb 12, 2025

Deploy Preview for hardcore-allen-f5257d ready!

Name Link
🔨 Latest commit edc0320
🔍 Latest deploy log https://app.netlify.com/sites/hardcore-allen-f5257d/deploys/67ae823f76cc120008e333ef
😎 Deploy Preview https://deploy-preview-1539--hardcore-allen-f5257d.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@JackPGreen JackPGreen enabled auto-merge (squash) February 13, 2025 09:05
@@ -613,7 +613,7 @@ In this step, you start a local instance of Management Center and use it to view
----
docker run \
--network hazelcast-network \
-p 8080:8080 hazelcast/management-center:latest-snapshot
-p 8080:8080 hazelcast/management-center:{page-latest-supported-mc}
Copy link
Contributor

@Rob-Hazelcast Rob-Hazelcast Feb 13, 2025

Choose a reason for hiding this comment

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

These don't appear to resolve when used in code samples. The original text wasn't a variable - was that just a mistake, or is latest-snapshot a valid input to Docker? If so, I think this change breaks the procedure?

@oliverhowell FYI, seems like we might need a solution for handling variables in code blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest snapshot isn’t a valid tag (presumably any more). I didn’t actually test it in the deployment though - we could use latest instead?

Copy link
Contributor Author

@JackPGreen JackPGreen Feb 13, 2025

Choose a reason for hiding this comment

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

Looks like I’m wrong and it works (weird as I’m sure I test locally!) - https://hub.docker.com/layers/hazelcast/management-center/latest-snapshot/images/sha256-5182aec7dfc667ade16b61ef2cb7b49e5936950af2b8a6d9fcbd4a564e474bd5

still shouldn’t be a snapshot tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And there is no latest… so if we can’t use substitution, maybe this is the best of a bad bunch?

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly. Could you check with someone in MC Engineering?

As a separate task, it might be worth us searching the docs for variables inside code blocks (regex in the source files?) and/or broken variables on the site (IDK).

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like attributes are filtered out by default (sensible), but you can add them back in? asciidoctor/asciidoctor#1741

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like attributes are filtered out by default (sensible), but you can add them back in? asciidoctor/asciidoctor#1741

Great spot!

edc0320

Possibly. Could you check with someone in MC Engineering?

I think this is a non-issue now the substitution works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a separate task, it might be worth us searching the docs for variables inside code blocks (regex in the source files?) and/or broken variables on the site (IDK).

Good idea, tried but could only find hazelcast/clc-docs#50

I'm surprised this issue doesn't occur more often tbh.

Here was how I tested it btw - just a simple set of (hacky) commands to build the docs locally, and scrape the output text for anything in curly braces and then log it + where it was found:

npm install
npm run-script build-local

find test -name "*.html" | while read -r file; do
    lynx -dump -listonly -nonumbers "${file}" | { grep --extended-regexp "\{[^}]*\}" && echo " in ${file}" || test $? = 1; }
done

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thank you for sorting all that out.

I think this is a non-issue now the substitution works?

I might still expect someone in the relevant team to sign off this sort of change, but the most important thing is that the updated command is tested and we're not breaking anything.

@@ -613,7 +613,7 @@ In this step, you start a local instance of Management Center and use it to view
----
docker run \
--network hazelcast-network \
-p 8080:8080 hazelcast/management-center:latest-snapshot
-p 8080:8080 hazelcast/management-center:{page-latest-supported-mc}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thank you for sorting all that out.

I think this is a non-issue now the substitution works?

I might still expect someone in the relevant team to sign off this sort of change, but the most important thing is that the updated command is tested and we're not breaking anything.

@JackPGreen JackPGreen merged commit 9ed728f into hazelcast:main Feb 14, 2025
5 of 6 checks passed
@JackPGreen JackPGreen deleted the Fix-MC-Docker-Links branch February 14, 2025 10:38
JackPGreen added a commit that referenced this pull request Feb 14, 2025
Backport of #1539

Update MC Docker tags to
`hazelcast/management-center:{page-latest-supported-mc}`.

Updated from:
- `latest-snapshot` - should use released artefacts
- `full-version` - the versions of Hazelcast platform and MC are not
tied (e.g. no `5.5.2` MC)

Co-authored-by: Jack Green <[email protected]>
JackPGreen added a commit that referenced this pull request Feb 14, 2025
Backport of #1539

Update MC Docker tags to
`hazelcast/management-center:{page-latest-supported-mc}`.

Updated from:
- `latest-snapshot` - should use released artefacts
- `full-version` - the versions of Hazelcast platform and MC are not
tied (e.g. no `5.5.2` MC)

Co-authored-by: Jack Green <[email protected]>
JackPGreen added a commit that referenced this pull request Feb 17, 2025
Backport of #1539

Update MC Docker tags to
`hazelcast/management-center:{page-latest-supported-mc}`.

Updated from:
- `latest-snapshot` - should use released artefacts
- `full-version` - the versions of Hazelcast platform and MC are not
tied (e.g. no `5.5.2` MC)

Co-authored-by: Jack Green <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport to all versions Backport commits to maintenance branches (from main)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants