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

Add endpoint copier operator component #45

Merged

Conversation

Kristian-ZH
Copy link
Contributor

Add endpoint-copier-operator package and define a GitHub action for it

@@ -0,0 +1,3 @@
url: https://github.com/suse-edge/endpoint-copier-operator.git
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we'll want to keep the chart in the endpoint-copier-operator repo long term, or move it into a package here similar to the other components?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping the chart in the endpoint-copier-operator repo was suggested in our Slack channel and the arguments were that in this way we keep the repo completed.
Akri follows the same approach.

servicetype: kubernetes-vip
spec:
ports:
- name: rke2-api
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail as cannot be two ports with the same name. I suggest to keep this one as rke2-api but the other one rename it as k8s-api

Copy link
Contributor

Choose a reason for hiding this comment

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

Also in this case, k3s doesn't need 9345, it is just rke2

Copy link
Contributor Author

@Kristian-ZH Kristian-ZH Oct 20, 2023

Choose a reason for hiding this comment

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

They do not have the same name actually (rke2-api and rke2-api2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also in this case, k3s doesn't need 9345, it is just rke2
The purpose of the test is to test if the endpoint-copier-operator will create the endpoint with two ports successfully.

The test just needs to create a Service with two random ports and see what will happen with the endpoint

Copy link
Contributor Author

@Kristian-ZH Kristian-ZH Oct 20, 2023

Choose a reason for hiding this comment

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

I will change the name of the ports to port1 and port2 just to not make any confusions

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I misread it.

Copy link
Contributor

@e-minguez e-minguez left a comment

Choose a reason for hiding this comment

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

One of the service ports needs to be modified

@Kristian-ZH Kristian-ZH force-pushed the add-endpoint-copier-operator-component branch from d555760 to 4c36aaf Compare October 20, 2023 09:14
@e-minguez e-minguez merged commit 63795a7 into suse-edge:main Oct 20, 2023
2 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