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

Fix Issue #196: Filtered the workflow context sent to worker by server #216

Merged
merged 3 commits into from
Aug 4, 2020

Conversation

parauliya
Copy link
Contributor

@parauliya parauliya commented Jul 13, 2020

Description

There are following change in this PR:

  1. Filtered the workflow context sent to worker by server on the basis of status and worker_id.
  2. Changed Info logs to Debug during sleep in worker side
  3. Changed a bit of logic in worker for not repeating the same workflow execution again.

Following things are fixed in second commit of this PR

  1. Modified the filter from the server side while sending the workflow context
  2. Added a gRPC stream for getting the workflow context
  3. Continue the loop for worker even after completing the workflow so that it can fetch any new workflow.

Why is this needed

This resolves the following issue.
Fixes: #196 #222 #72

How Has This Been Tested?

This has been tested by creating a sample worklfow and execute it.
Following scenarios was executed for the testing:

  1. One workflow has one worker
  2. One workflow has two workers
  3. Two workflows and each workflow has two workers

How are existing users impacted? What migration steps/scripts do we need?

It will enhance the user experience as logging will be lesser as compared to the current one on worker side.
There is no migration script needed.

@parauliya parauliya force-pushed the fix_196 branch 2 times, most recently from e7aff1a to 207bfbf Compare July 17, 2020 07:45
@parauliya parauliya self-assigned this Jul 17, 2020
@parauliya parauliya added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 17, 2020
executor/executor.go Show resolved Hide resolved
Copy link
Contributor

@nathangoulding nathangoulding left a comment

Choose a reason for hiding this comment

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

We still need to handle the scenarios where we have an already connected worker/stream, and 1) a new workflow gets created that targets that worker, or 2) an already executing workflow reaches a step where the worker gets new work. How is this achieved in this situation?

executor/executor.go Outdated Show resolved Hide resolved
executor/executor.go Show resolved Hide resolved
protos/workflow/workflow.proto Show resolved Hide resolved
@parauliya
Copy link
Contributor Author

parauliya commented Jul 30, 2020

We still need to handle the scenarios where we have an already connected worker/stream, and 1) a new workflow gets created that targets that worker, or 2) an already executing workflow reaches a step where the worker gets new work. How is this achieved in this situation?

This handles both of your cases as follows:

  1. When the worker do the iPXE boot for the first time, it first executes all the existing workflow which are targeted to it and then it will keep on running on that worker machine. And If you create a new workflow which is targeted to this worker then that workflow will be executed automatically.
  2. Worker will first execute all the workflows which were created before it was started and then it will go for the new workflows which are targated to it.

…r by server

There is a bit of change in worker as well:
1. Changed Info logs to Debug during sleep
2. Changed a bit of logic of starting the workflow action to execute
Following things are fixed:
1. Modified the filter from the server side while sending the workflow context
2. Added a gRPC stream for getting the workflow context
3. Continue the loop for worker even after completing the workflow
@nathangoulding
Copy link
Contributor

This handles both of your cases as follows:

  1. When the worker do the iPXE boot for the first time, it first executes all the existing workflow which are targeted to it and then it will keep on running on that worker machine. And If you create a new workflow which is targeted to this worker then that workflow will be executed automatically.
  2. Worker will first execute all the workflows which were created before it was started and then it will go for the new workflows which are targated to it.

You're right. When we add the ability for workers to fetch-and-wait instead of fetch-and-quit, as addressed in issue #222, we will need to update this so that the server sends work to the workers that are already connected and waiting.

@parauliya parauliya merged commit c0a4a29 into tinkerbell:master Aug 4, 2020
Issues List automation moved this from In Progress to Just shipped Aug 4, 2020
@parauliya parauliya deleted the fix_196 branch September 1, 2020 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
No open projects
Issues List
  
Just shipped
3 participants