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

manager: should not use calico mac address to check the node #50

Merged

Conversation

Vicente-Cheng
Copy link
Collaborator

- We depend on the system interface to ensure the node ID, but the calico MAC address `ee:ee:ee:ee:ee:ee` is the special MAC address that each guest cluster might have. So, we should not use it to check the node id.

IMPORTANT: Please do not create a Pull Request without creating an issue first.

Problem:
The Harvester CSI driver used the wrong node ID, which caused the volume to attach to the wrong node.
Then, every workload that depends on that volume will hang.

Solution:
Do not use the Calico mac address to check the node.

Related Issue:
harvester/harvester#6981

Test plan:

    - We depend on the system interface to ensure the node ID, but the
      calico MAC address `ee:ee:ee:ee:ee:ee` is the special MAC address
      that each guest cluster might have. So, we should not use it to
      check the node id.

Signed-off-by: Vicente Cheng <[email protected]>
Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

we can also create a CLI flag for the exception list so the user will have a chance to modify it in a running env.
But this lgtm for a quick fix. Thanks

Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR.

@WebberHuang1118 WebberHuang1118 merged commit 7cd9bf9 into harvester:master Nov 12, 2024
4 checks passed
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.

3 participants