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

🌱 TMC is live! #2

Merged
merged 19 commits into from
Jul 24, 2023
Merged

🌱 TMC is live! #2

merged 19 commits into from
Jul 24, 2023

Conversation

mjudeikis
Copy link
Contributor

@mjudeikis mjudeikis commented Jul 9, 2023

In this PR:

  • adding basic CI using github actions for now
  • Controllers refactored to use separate informers
  • Regenerated new clients
  • use Loopback client for TMC resources
  • Adds new tmc workspaceType for TMC specific

Important:

@mjudeikis
Copy link
Contributor Author

Before we open this to the world I would like to get e2e running as currently they are failing. But failures looks like more of the migration, not code.

@fgiloux
Copy link
Contributor

fgiloux commented Jul 10, 2023

I have tried to run locally and the syncer is able to connect to kcp and to update the status.
When trying to bind the kubernetes APIExport and to create a deployment it is working in root:kubernetes but not in another workspace: kvdk2spgmbix:test-sync. I get the following error:

kubectl kcp bind compute root:compute:kubernetes
placement placement-jsym7slt created.
error: placement "placement-jsym7slt" not ready: placement is not scheduled: No selected location is scheduled

It is possible that I am simply missing a step.

There are a few minor things in Makefile:

  • make generate -> it should call make tools to get apigen installed if needed
  • make build-> it should create the bin directory if it does not exist

I will try to look at the code changes when time allows.

@mjudeikis
Copy link
Contributor Author

When trying to bind the kubernetes APIExport and to create a deployment it is working in root:kubernetes but not in another workspace: kvdk2spgmbix:test-sync. I get the following error:

I think it created default placement, and if there are no default location (no synctarget present) it will fail. This is what I found in the code before, and didn't changed.

Did you had synctarget in the test-sync workspace?

Copy link
Contributor

@fgiloux fgiloux left a comment

Choose a reason for hiding this comment

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

A few shallow comments. I will do a bit more testing

Makefile Outdated Show resolved Hide resolved
# Build CLI binaries

make build

Copy link
Contributor

Choose a reason for hiding this comment

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

kubectl-tmc and kubectl-workload need to be copied to the kubectl directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added it into readme to add to the path or copy. The path is secured on most distros, so this would require higher privileges :/

README.md Outdated Show resolved Hide resolved
cmd/kubectl-tmc/cmd/kubectlTmc.go Outdated Show resolved Hide resolved
cmd/kubectl-tmc/cmd/kubectlTmc.go Outdated Show resolved Hide resolved
cmd/tmc/tmc.go Outdated Show resolved Hide resolved
@fgiloux
Copy link
Contributor

fgiloux commented Jul 12, 2023

Log forwarding is not working but that's something we can look at later on:

$ kubectl get pods
NAME                     READY   STATUS    RESTARTS   AGE
kuard-587f7fc9cc-ptdst   1/1     Running   0          46s
[fgiloux@fgiloux contrib-tmc]$ kubectl logs kuard-587f7fc9cc-ptdst 
Error from server (NotFound): pods "kuard-587f7fc9cc-ptdst" not found

@fgiloux
Copy link
Contributor

fgiloux commented Jul 12, 2023

I am still facing this issue. I will try to troubleshoot it tomorrow

@mjudeikis
Copy link
Contributor Author

Yes! Now I remember:

// TODO(MJ): currently this disables the cross workspace placements. This is due to
	// fact indexers are build ontop of logicalcluster paths and we use here the 'readable' paths
	// if len(placement.Spec.LocationWorkspace) > 0 {
	//	locationWorkspace = logicalcluster.NewPath(placement.Spec.LocationWorkspace)
	// } else {
	locationWorkspace = logicalcluster.From(placement).Path()
	// }

Will create an issue to follow-up with it

@mjudeikis
Copy link
Contributor Author

Currently, it works for basic deploy container to pcluster.
No sharding, no cross-workspace locations/placements,no pod logs.
All of these things would have to come with VM refactoring. But there are big limitations on the current approach due to how KCP setups informers (when in the lifecycle) and if you extend you can't use same patterns. In addition, the caching server is not working for 3rd party resource as it expects it to be using workspaces/apiexports.

I think we need to merge this, and start working on VW implementation where we would be deploying ontop of KCP fully. As feel like we just need rip the cord and move on with this.

@mjudeikis mjudeikis merged commit 59649fa into main Jul 24, 2023
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.

2 participants