[FLINK-28829][k8s] Support prepreparing K8S resources before JM creation #20498

bzhaoopenstack · 2022-08-08T11:46:16Z

In this PR, we introduces several things for supporting create another k8s resources before JM deploy creation

What is the purpose of the change

(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)
For supporting customized k8s scheduler, we need to extend the ability of current code tree to support the cases of all customized k8s scheduler during they schedule the real pods. So we'd better to extend all cases during k8s scheduling, such as before/during/after our target resources creation. In this PR, we add the before case. And current code tree had covered during/after cases.

Brief change log

We introduce a new interface func for all decorators to support
create pre-prepared k8s resources.
We extend a attribute for JM spec for storing the resources list.
Extending the ability for supporting refresh the preprepared
resource's ownerreference based on original code logic.
Add the ability for creating K8S resource before JM deployment
creation in Fabric client.

Verifying this change

We can not verify this change at this moment, as this is a internal process in flink-kubernetes now. It doesn't affect the user interface now. The only way we can test it is mocking a JM spec with another k8s resources which is not contained by Flink. And check whether the resources can be created before the JM and refresh the correct ower reference(Flink cluster id-- deployment id).

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
The serializers: (no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (no)
If yes, how is the feature documented? (not documented)

flinkbot · 2022-08-08T12:53:36Z

CI report:

e9939ec Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

martin-g · 2022-10-10T06:31:06Z

.../src/main/java/org/apache/flink/kubernetes/kubeclient/KubernetesJobManagerSpecification.java

@@ -30,10 +30,15 @@ public class KubernetesJobManagerSpecification {

    private List<HasMetadata> accompanyingResources;

+    private List<HasMetadata> prePreparedResources;


Suggested change

private List<HasMetadata> prePreparedResources;

private final List<HasMetadata> prePreparedResources;

hmm, how about just follow the style of accompanyingResources? If we change this, we should also keep the same with accompanyingResources. And that was not related with this PR.

...ernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClientTest.java

...est/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactoryTest.java

martin-g · 2022-10-10T06:46:27Z

...ernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClientTest.java

+
+        // check the preprepared resources had been created.
+        final List<Pod> resultPods = kubeClient.pods().inNamespace(NAMESPACE).list().getItems();
+        assertThat(resultPods).hasSize(3);


Shouldn't this be at least 4 pods ?
1 pod for the JM itself and 3 mock-pod-preprepared-resource-**.

This is UT, it won't create any real Pods, just a k8s models objects. Actually, JM will be created by K8S deployment, In the real env, that's true it should be 4 pods.

In this PR, we introduces several things below: 1. We introduce a new interface func for all decorators to support create pre-prepared k8s resources. 2. We extend a attribute for JM spec for storing the resources list. 3. Extending the ability for supporting refresh the preprepared resource's ownerreference based on original code logic. 4. Add the ability for creating K8S resource before JM deployment creation in Fabric client.

bzhaoopenstack · 2022-10-11T00:55:20Z

@flinkbot run azure

wangyang0918

Thanks for creating this PR. After more consideration, I have one concern about whether we really need the pre-prepared resources. If the accompanyingResources are created before the JobManager deployment, then it could satisfy our requirement. Right?

FYI: accompanyingResources is designed for being created before deployment. The current implementation is just to avoid resource leak and save one K8s apiserver request.

wangyang0918 · 2022-10-19T05:18:02Z

...-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClient.java

+        final List<HasMetadata> prePreparedResources = kubernetesJMSpec.getPrePreparedResources();
+
+        // before create Deployment
+        this.internalClient.resourceList(prePreparedResources).createOrReplace();


We might have resources leak here if the flink client crashed exactly after the pre-prepared resources created.

wangyang0918 · 2022-10-19T05:21:42Z

...ernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClientTest.java

@@ -630,4 +630,37 @@ private KubernetesConfigMap buildTestingConfigMap() {
                        .withData(data)
                        .build());
    }
+
+    @Test
+    void testMockPrePreparedResources() throws Exception {


The throws Exception is useless.

jrfaller · 2022-11-22T14:14:37Z

Hi! As you probably know, this PR is introducing some breaking changes. Our academic tool BreakBot helps developers to better assess the impact that these BCs may have on client projects. While this can be on purpose, we found out that some clients among the most popular on GitHub may be faced with broken code.
You can find the full BreakBot report in our fork repository: report for PR.

We hope this information is valuable to you, and apologize otherwise.
If you're willing to help, we would kindly ask for your help to fill in a 5-minutes survey about the report.
Your feedback will help us improve the tool and help us in our research!

bzhaoopenstack changed the title ~~[FLINK-28829] Support prepreparing K8S resources before JM creation~~ [FLINK-28829][k8s] Support prepreparing K8S resources before JM creation Aug 8, 2022

flinkbot added the component=Deployment/Kubernetes label Aug 8, 2022

bzhaoopenstack force-pushed the upstream-precreate-k8s-resource branch from e6e33f8 to 5d9c834 Compare August 19, 2022 06:23

bzhaoopenstack force-pushed the upstream-precreate-k8s-resource branch from 5d9c834 to 211d0ef Compare September 29, 2022 11:34

martin-g reviewed Oct 10, 2022

View reviewed changes

bzhaoopenstack force-pushed the upstream-precreate-k8s-resource branch from 211d0ef to e9939ec Compare October 10, 2022 11:24

wangyang0918 requested changes Oct 19, 2022

View reviewed changes

wangyang0918 self-assigned this Oct 19, 2022

gj409237405 mentioned this pull request Feb 23, 2023

[FLINK-28825][k8s] Add K8S pod scheduler into Kubernetes options #20467

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-28829][k8s] Support prepreparing K8S resources before JM creation #20498

[FLINK-28829][k8s] Support prepreparing K8S resources before JM creation #20498

bzhaoopenstack commented Aug 8, 2022

flinkbot commented Aug 8, 2022 •

edited

Loading

martin-g Oct 10, 2022

bzhaoopenstack Oct 10, 2022 •

edited

Loading

martin-g Oct 10, 2022

bzhaoopenstack Oct 10, 2022 •

edited

Loading

bzhaoopenstack commented Oct 11, 2022

wangyang0918 left a comment

wangyang0918 Oct 19, 2022

wangyang0918 Oct 19, 2022

jrfaller commented Nov 22, 2022

		@@ -30,10 +30,15 @@ public class KubernetesJobManagerSpecification {

		private List<HasMetadata> accompanyingResources;

		private List<HasMetadata> prePreparedResources;

	private List<HasMetadata> prePreparedResources;
	private final List<HasMetadata> prePreparedResources;

[FLINK-28829][k8s] Support prepreparing K8S resources before JM creation #20498

Are you sure you want to change the base?

[FLINK-28829][k8s] Support prepreparing K8S resources before JM creation #20498

Conversation

bzhaoopenstack commented Aug 8, 2022

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Aug 8, 2022 • edited Loading

CI report:

martin-g Oct 10, 2022

Choose a reason for hiding this comment

bzhaoopenstack Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

martin-g Oct 10, 2022

Choose a reason for hiding this comment

bzhaoopenstack Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

bzhaoopenstack commented Oct 11, 2022

wangyang0918 left a comment

Choose a reason for hiding this comment

wangyang0918 Oct 19, 2022

Choose a reason for hiding this comment

wangyang0918 Oct 19, 2022

Choose a reason for hiding this comment

jrfaller commented Nov 22, 2022

flinkbot commented Aug 8, 2022 •

edited

Loading

bzhaoopenstack Oct 10, 2022 •

edited

Loading

bzhaoopenstack Oct 10, 2022 •

edited

Loading