Skip to content

fix(ci): adds nodejs 20 to ci-container#2536

Merged
onur-ozkan merged 4 commits intodevfrom
add-nodejs-to-ci-container
Jul 18, 2025
Merged

fix(ci): adds nodejs 20 to ci-container#2536
onur-ozkan merged 4 commits intodevfrom
add-nodejs-to-ci-container

Conversation

@smk762
Copy link
Copy Markdown

@smk762 smk762 commented Jul 18, 2025

While working on updates to the notary seed node repo, I noticed https://hub.docker.com/r/komodoofficial/komodo-defi-framework/tags did not appear to be updating with the most recent releases.

The cause of this was found to be failing GHA runs due to a lack of nodejs. To resolve this, I added an install step into the ci-container and pushed an update to dockerhub as I couldn't see an automated pipeline for that in this repo.

Once this was done, previously failing builds in release-build.yml were successful.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This file is used to create image that CI runners use it to build KDF (which is manually pushed only when needed). You are probably looking for Dockerfile.dev-release (for dev builds) and Dockerfile.release (for main builds) ones.

@smk762
Copy link
Copy Markdown
Author

smk762 commented Jul 18, 2025

The failure to build was due to https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/.github/actions/build-cache/action.yml#L10 requiring nodejs.

This is referenced in a variety of CI files
image

And the ci-container is used in the workflows which build the binaries
image

The option for resolving all without updating many files, was to either place it in the https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/.docker/Dockerfile.ci-container, or in https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/.github/actions/build-cache/action.yml

Adding the nodejs to https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/.docker/Dockerfile.dev-release or https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/.docker/Dockerfile.release would not work, because the CI build would fail before it even got there.

The end result (after building the ci-container and pushing it to dockerhub locally) was all green lights in https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/16362722703 so on the surface at least, it appears to have the desired effect.

@smk762
Copy link
Copy Markdown
Author

smk762 commented Jul 18, 2025

In testing on a re-run with https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/16319503901/job/46235718645#step:14:62 the build was successful, though it still failed to complete due to old refs to mm2. I've removed them in here also, to avoid that error in future actions runs.

@onur-ozkan
Copy link
Copy Markdown

@smk762
Copy link
Copy Markdown
Author

smk762 commented Jul 18, 2025

I did that first, then thought container would have better coverage in case node was required elsewhere. I've switched back so its more focused on the action which requires it and keeps ci-container a little lighter. If you prefer this, I'll leave it that way, and update the ci-container on dockerhub again to remove the nodejs layer.

Copy link
Copy Markdown

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM

I did that first, then thought container would have better coverage in case node was required elsewhere. I've switched back so its more focused on the action which requires it and keeps ci-container a little lighter. If you prefer this, I'll leave it that way, and update the ci-container on dockerhub again to remove the nodejs layer.

I think it makes more sense to keep CI container as light and dep-agnostic (from the 3rd parties) as possible.

@onur-ozkan onur-ozkan merged commit b59860b into dev Jul 18, 2025
17 of 24 checks passed
@onur-ozkan onur-ozkan deleted the add-nodejs-to-ci-container branch July 18, 2025 07:41
dimxy pushed a commit that referenced this pull request Aug 1, 2025
* dev: (21 commits)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  improve note for docker test start failure (#2550)
  fix(DOCS): add note for macos to fix docker containers startup failure (#2544)
  refactor(toolchain): general stabilization for stable rust (#2528)
  fix(ci): adds nodejs 20 to ci-container (#2536)
  fix(WASM and Debian): fix build failures (#2534)
  improvement(event-streaming): impl DeriveStreamerId trait for all streamers (#2489)
  fix(eth): Propagate structured EIP-1559 fee errors (#2532)
  fix(eth): Correctly implement ETH max withdrawal logic (#2531)
  feat(use-clap-for-cli): use clap to parse CLI-Args #2215 (#2510)
  feat(orderbook): expirable maker orders (#2516)
  improvement(eth): drop parity support (#2527)
  chore(release): finalize changelog for v2.5.0-beta (#2524)
  chore(toolchain): upgrade toolchain to nightly 1.86.0 (#2444)
  ...

# Conflicts:
#	mm2src/coins/lp_coins.rs
#	mm2src/coins/rpc_command/get_new_address.rs
#	mm2src/trezor/src/eth/eth_command.rs
dimxy pushed a commit that referenced this pull request Aug 1, 2025
* dev:
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  improve note for docker test start failure (#2550)
  fix(DOCS): add note for macos to fix docker containers startup failure (#2544)
  refactor(toolchain): general stabilization for stable rust (#2528)
  fix(ci): adds nodejs 20 to ci-container (#2536)
  fix(WASM and Debian): fix build failures (#2534)
  improvement(event-streaming): impl DeriveStreamerId trait for all streamers (#2489)
  chore(release): v2.3.0-beta (#2284)

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_tests.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Oct 15, 2025
* adds nodejs 20 to ci container

* remove comment

* remove mm2 (in favor of kdf) from dockerfiles

* move node install from ci container to build cache action
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants