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

chore(local-dev): add node affinity on the patches for local dev with EC #5002

Merged
merged 9 commits into from
Nov 20, 2024

Conversation

JGAntunes
Copy link
Member

@JGAntunes JGAntunes commented Nov 13, 2024

What this PR does / why we need it:

When running local dev with embedded cluster, running make kotsadm-up-ec will build the local dev images and load them into node0 (by default). If we're running a multi node cluster setup it so happens that the scheduled pods might land in any of the other nodes other than node0 (which won't have the loaded image). This affinity ensures we always schedule the pods on the correct node we set using EC_NODE env var.

Which issue(s) this PR fixes:

NONE

Does this PR require a test?

NONE

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

@sgalsaleh
Copy link
Member

You can use the EC_NODE env var to connect to a different node than node0:

export EC_NODE=node1

This is because you can have multiple clusters and kotsadm should not necessarily always run on node0.

@JGAntunes
Copy link
Member Author

JGAntunes commented Nov 13, 2024

You can use the EC_NODE env var to connect to a different node than node0:

Right but even if you do, for this to work, you need to run make kotsadm-up-ec for each node so that ec_build_and_load loads the image into all the cluster nodes:

  • # Build and load the image into the embedded cluster
    ec_build_and_load "$component"
  • kots/dev/scripts/common.sh

    Lines 106 to 112 in 55b35f4

    # Load the image into the embedded cluster
    if docker exec $(ec_node) k0s ctr images ls | grep -q "$(image $1)"; then
    echo "$(image $1) image already loaded in embedded cluster, skipping import..."
    else
    echo "Loading "$(image $1)" image into embedded cluster..."
    docker save "$(image $1)" | docker exec -i $(ec_node) k0s ctr images import -
    fi

Else the deployment will likely fail with pull image backoff errors if the pod lands on a node where the image isn't available.

I thought it would just be easier to pin the deployment to a single node when doing local dev as way to overcome this.

@sgalsaleh
Copy link
Member

How about we pin it to the value of EC_NODE then? or node0 if EC_NODE is not specified? Also, need to do this for kurl-proxy as well.

@JGAntunes
Copy link
Member Author

How about we pin it to the value of EC_NODE then? or node0 if EC_NODE is not specified? Also, need to do this for kurl-proxy as well.

Ah got it, yeah that makes sense. I can look into that 👍

@JGAntunes
Copy link
Member Author

Done @sgalsaleh. Let me know what you think.

@JGAntunes JGAntunes changed the title chore(local-dev): add node0 affinity on the patches for local dev with EC chore(local-dev): add node affinity on the patches for local dev with EC Nov 14, 2024
@sgalsaleh
Copy link
Member

Actually, this same patch is used for existing cluster (vanilla KOTS dev env), which I don't believe will work as the node is not named the same way.

@JGAntunes
Copy link
Member Author

Actually, this same patch is used for existing cluster (vanilla KOTS dev env), which I don't believe will work as the node is not named the same way.

Good catch. Went with a different approach, let me know what you think.

Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@JGAntunes JGAntunes enabled auto-merge (squash) November 20, 2024 10:42
@JGAntunes JGAntunes merged commit 3d37fe7 into main Nov 20, 2024
122 checks passed
@JGAntunes JGAntunes deleted the chore/ec-dev-patches-affinity branch November 20, 2024 17:03
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.

2 participants