Add fake-source-cluster for testing the sync service #364

saza-ku · 2024-08-20T09:32:37Z

What type of PR is this?

/area simulator
/kind feature

What this PR does / why we need it:

This PR adds a kwok cluster that will be the source cluster of import/sync when debugging.
This cluster starts by running make docker_debug.

Which issue(s) this PR fixes:

Part of #327

Special notes for your reviewer:

When you edit simulator/config.yaml to enable externalImportEnabled and run make docker_debug, you will see node-0, node-1 and node-2 in the simulator UI.

/label tide/merge-method-squash

k8s-ci-robot · 2024-08-20T09:32:46Z

Hi @saza-ku. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

docker-compose-local.yml

sanposhiho · 2024-08-20T11:18:28Z

docker-compose-local.yml

@@ -49,6 +53,38 @@ services:
      - KWOK_KUBE_APISERVER_PORT=3131
    networks:
      - simulator-internal-network
+  debug-real-cluster:


Suggested change

debug-real-cluster:

fake-real-cluster:

I agree that "fake" is more appropriate than "debug".
"debug" seems to overly narrow the intended purpose of the cluster.

However, "fake" and "real" words are contradictory.
How about "fake-source-cluster"?

sanposhiho · 2024-08-20T12:09:04Z

Makefile

@@ -68,10 +68,18 @@ docker_up_local:
 .PHONY: docker_build_and_up
 docker_build_and_up: docker_build docker_up_local

+.PHONY: docker_debug


I'm not sure we should add a new make command.
Can we implement it as an option in the existing docker_up_local, like:

docker compose up -d --profile $(profiles)

We can specify the profiles by an environment variable of COMPOSE_PROFILES, like this.

make docker_up_local -e COMPOSE_PROFILES=externalImportEnabled

So this PR doesn't fix Makefile.

simulator/cmd/simulator/Dockerfile

docker-compose-local.yml

sanposhiho · 2024-08-20T12:27:15Z

/ok-to-test

ordovicia · 2024-08-21T00:47:03Z

docker-compose-local.yml

@@ -49,6 +53,38 @@ services:
      - KWOK_KUBE_APISERVER_PORT=3131
    networks:
      - simulator-internal-network
+  debug-real-cluster:


I agree that "fake" is more appropriate than "debug".
"debug" seems to overly narrow the intended purpose of the cluster.

However, "fake" and "real" words are contradictory.
How about "fake-source-cluster"?

docker-compose-local.yml

…as volumes

ordovicia · 2024-08-21T05:55:22Z

Could you please also update simulator/docs/running-simulator.md to describe how to run the fake source cluster?

saza-ku · 2024-08-21T06:18:58Z

Could you please also update simulator/docs/running-simulator.md to describe how to run the fake source cluster?

Updated!

ordovicia

👍

simulator/docs/running-simulator.md

docker-compose-local.yml

Co-authored-by: Kensei Nakada <[email protected]>

sanposhiho · 2024-08-22T01:08:02Z

docker-compose.yml

@@ -3,6 +3,9 @@ services:
  simulator-server:
    image: registry.k8s.io/scheduler-simulator/simulator-backend:v0.2.0
    container_name: simulator-server
+    volumes:
+      - ./simulator/config.yaml:/config.yaml
+      - ./simulator/kubeconfig.yaml:/kubeconfig.yaml


We shouldn't mount this kubeconfig since we don't have fake-source-cluster in this case.

sanposhiho

/lgtm
/approve

k8s-ci-robot · 2024-08-22T02:42:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ordovicia, sanposhiho, saza-ku

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sanposhiho]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…#364) * add debug-real-cluster * delete init-debug-real-cluster * rename debug profile to externalImportEnabled * rename debug-real-cluster to fake-source-cluster * delete docker_debug and docker_down_debug * delete config.yaml and kubeconfig.yaml from the image and mount them as volumes * add description of how to run fake-source-cluster * fix description of how to run fake-source-cluster Co-authored-by: Kensei Nakada <[email protected]> * mount config.yaml and in docker-compose.yml --------- Co-authored-by: Kensei Nakada <[email protected]>

add debug-real-cluster

d1abefe

k8s-ci-robot requested review from 196Ikuchil and Huang-Wei August 20, 2024 09:32

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 20, 2024

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 20, 2024

saza-ku mentioned this pull request Aug 20, 2024

simulator: synchronically replicate cluster state from a real cluster to a fake one for the scheduler testing #327

Closed

sanposhiho reviewed Aug 20, 2024

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 20, 2024

ordovicia reviewed Aug 21, 2024

View reviewed changes

saza-ku added 5 commits August 21, 2024 13:58

delete init-debug-real-cluster

8414133

rename debug profile to externalImportEnabled

6d6dafd

rename debug-real-cluster to fake-source-cluster

0060a3f

delete docker_debug and docker_down_debug

fa791e7

delete config.yaml and kubeconfig.yaml from the image and mount them …

e9a67e5

…as volumes

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 21, 2024

saza-ku force-pushed the add-test-real-cluster branch from 2692456 to e9a67e5 Compare August 21, 2024 05:38

saza-ku requested review from ordovicia and sanposhiho August 21, 2024 05:42

saza-ku changed the title ~~Add debug-real-cluster for testing the import service and the sync service~~ Add fake-source-cluster for testing the sync service Aug 21, 2024

add description of how to run fake-source-cluster

f4112c0

ordovicia approved these changes Aug 21, 2024

View reviewed changes

sanposhiho reviewed Aug 21, 2024

View reviewed changes

simulator/docs/running-simulator.md Outdated Show resolved Hide resolved

docker-compose-local.yml Show resolved Hide resolved

fix description of how to run fake-source-cluster

4ceb7b4

Co-authored-by: Kensei Nakada <[email protected]>

saza-ku force-pushed the add-test-real-cluster branch from 68c71c4 to 5ab5224 Compare August 22, 2024 01:01

saza-ku requested a review from sanposhiho August 22, 2024 01:02

sanposhiho reviewed Aug 22, 2024

View reviewed changes

saza-ku force-pushed the add-test-real-cluster branch 2 times, most recently from 4a7a4cb to 38c9288 Compare August 22, 2024 02:37

mount config.yaml and in docker-compose.yml

d6cd2d9

saza-ku force-pushed the add-test-real-cluster branch from 38c9288 to d6cd2d9 Compare August 22, 2024 02:38

saza-ku requested a review from sanposhiho August 22, 2024 02:38

sanposhiho approved these changes Aug 22, 2024

View reviewed changes

k8s-ci-robot assigned sanposhiho Aug 22, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 22, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 22, 2024

k8s-ci-robot merged commit dad23a8 into kubernetes-sigs:master Aug 22, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fake-source-cluster for testing the sync service #364

Add fake-source-cluster for testing the sync service #364

saza-ku commented Aug 20, 2024

k8s-ci-robot commented Aug 20, 2024

sanposhiho Aug 20, 2024

ordovicia Aug 21, 2024

sanposhiho Aug 20, 2024

saza-ku Aug 21, 2024

sanposhiho commented Aug 20, 2024

ordovicia Aug 21, 2024

ordovicia commented Aug 21, 2024

saza-ku commented Aug 21, 2024 •

edited

Loading

ordovicia left a comment

sanposhiho Aug 22, 2024

sanposhiho left a comment

k8s-ci-robot commented Aug 22, 2024

Add fake-source-cluster for testing the sync service #364

Add fake-source-cluster for testing the sync service #364

Conversation

saza-ku commented Aug 20, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

k8s-ci-robot commented Aug 20, 2024

sanposhiho Aug 20, 2024

Choose a reason for hiding this comment

ordovicia Aug 21, 2024

Choose a reason for hiding this comment

sanposhiho Aug 20, 2024

Choose a reason for hiding this comment

saza-ku Aug 21, 2024

Choose a reason for hiding this comment

sanposhiho commented Aug 20, 2024

ordovicia Aug 21, 2024

Choose a reason for hiding this comment

ordovicia commented Aug 21, 2024

saza-ku commented Aug 21, 2024 • edited Loading

ordovicia left a comment

Choose a reason for hiding this comment

sanposhiho Aug 22, 2024

Choose a reason for hiding this comment

sanposhiho left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 22, 2024

saza-ku commented Aug 21, 2024 •

edited

Loading