Build by t-aretz · Pull Request #2 · CommonWorkflowScheduler/KubernetesScheduler

t-aretz · 2023-10-04T10:20:21Z

No description provided.

Lehmann-Fabian

Thanks for this contribution. Please remove all the logs or change the log level.
Please address/answer the other remarks.
And document somewhere how to use this feature.

src/main/java/cws/k8s/scheduler/model/TaskInput.java

src/main/java/cws/k8s/scheduler/rest/SchedulerRestController.java

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

Lehmann-Fabian · 2023-10-06T12:28:58Z

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

+
+        // print Tasks
+        System.out.println("Tasks before Label Alignment");
+        unscheduledTasks.stream().map(obj -> obj.getConfig().getName()).forEach(System.out::println);


Do not log this in stable version

Now you use Stdout.
Do not log this at all, or log at debug level if you need

src/main/java/cws/k8s/scheduler/scheduler/nodeassign/LabelAssign.java

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

Lehmann-Fabian · 2023-11-07T15:58:32Z

src/main/java/cws/k8s/scheduler/rest/SchedulerRestController.java

 import java.util.List;
 import java.util.Map;

+import com.fasterxml.jackson.databind.ObjectMapper;


Please remove this unused import

Lehmann-Fabian · 2023-11-07T15:58:57Z

src/main/java/cws/k8s/scheduler/rest/SchedulerRestController.java

+        // ObjectMapper objetMapper = new ObjectMapper();
+        // Map<String,String> nodelabel = objectMapper.convertValue(config.additional.get("tasklabelconfig"),Map.class);


Remove this

Lehmann-Fabian · 2023-11-07T16:00:04Z

src/main/java/cws/k8s/scheduler/rest/SchedulerRestController.java

-                Prioritize prioritize;
-                NodeAssign assign;


You can undo this change.

Or make prioritize and assign configurable

Lehmann-Fabian · 2023-11-07T16:04:19Z

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

+
+        // print Tasks
+        System.out.println("Tasks before Label Alignment");
+        unscheduledTasks.stream().map(obj -> obj.getConfig().getName()).forEach(System.out::println);


Now you use Stdout.
Do not log this at all, or log at debug level if you need

Lehmann-Fabian · 2023-11-07T16:06:34Z

src/main/java/cws/k8s/scheduler/scheduler/Scheduler.java

        }
        logInfo.add("------------------------------------");
        log.info(String.join("\n", logInfo));
+


I think you can leave this class as it was.

Lehmann-Fabian · 2023-11-07T16:13:58Z

src/main/java/cws/k8s/scheduler/scheduler/nodeassign/LabelAssign.java

+            }            
+
+            final PodWithAge pod = task.getPod();
+            // log.info("Pod: " + pod.getName() + " Requested Resources: " + pod.getRequest() );


Lehmann-Fabian · 2023-11-07T16:16:18Z

src/main/java/cws/k8s/scheduler/scheduler/nodeassign/LabelAssign.java

+                for ( Map.Entry<NodeWithAlloc, Requirements> e : availableByNode.entrySet() ) {
+                    final NodeWithAlloc node = e.getKey();
+
+                    if(nodeName.equals(node.getName())){
+                        log.info("Aligned Pod to node: " + node.getName());
+                        alignment.add( new NodeTaskAlignment( node, task ) );
+                        availableByNode.get( node ).subFromThis(pod.getRequest());
+                        log.info("--> " + node.getName());
+                        task.getTraceRecord().foundAlignment();
+                        break;
+                    } 
+                }


Please request this by the key and do not iterate the Map

Lehmann-Fabian · 2023-11-07T16:18:56Z

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

+public class NodeLabelAssign extends Scheduler {
+
+    private final Prioritize prioritize;
+    private final NodeAssign nodeAssigner;


Maybe name this defaultNodeAssigner or fallbackNodeAssigner to clearify its function

Lehmann-Fabian · 2023-11-07T16:21:34Z

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

+
+        // first alignemnt (LabelAssign)
+        List<NodeTaskAlignment> alignmentLabelAssign = nodeLabelAssigner.getTaskNodeAlignment(unscheduledTasks, availableByNode);
+        List<String> namesList = alignmentLabelAssign.stream().map(obj -> obj.task.getConfig().getName()).collect(Collectors.toList());


listOfUnassignedTasks

I think it is better to make variable alignmentLabelAssign from type NodeLabelAssign and then store a list of unscheduled tasks and fetch it here. Then you don't need to calculate this here again.

Lehmann-Fabian · 2023-11-07T16:25:36Z

src/main/java/cws/k8s/scheduler/scheduler/NodeLabelAssign.java

+            }
+        }
+
+        // second alignemnt (FairAssign)


alignemnt -> alignment
Please add a more informative description:
Second Alignment: Here, all nodes are assigned that do not have a label.
This is not necessarily a FairAssign (The constructor can be called with different strategies)

t-aretz added 8 commits July 22, 2023 09:05

Test Own Scheduler Pod

736056f

Add CSV Reader

73cb083

Csv Update

a44461d

Tests

14c9e4f

Update

9305900

Update

e05cf3a

Should work

7415c5c

Update Last few things

0878b6e

Lehmann-Fabian requested changes Oct 6, 2023

View reviewed changes

t-aretz added 5 commits November 7, 2023 11:28

Some changes - still testing

b220902

switch to log.info

6c26c63

Error Message

fec8602

Break for add

fcd1e6e

changed to warn

ed55fd4

Lehmann-Fabian requested changes Nov 7, 2023

View reviewed changes

		// ObjectMapper objetMapper = new ObjectMapper();
		// Map<String,String> nodelabel = objectMapper.convertValue(config.additional.get("tasklabelconfig"),Map.class);

Comments

Conversation

t-aretz commented Oct 4, 2023

Uh oh!

Lehmann-Fabian left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Lehmann-Fabian left a comment •

edited

Loading