Sedna FederatedLearning controller enhancement #446
Conversation
kubeedge-bot
added
the
size/XXL
| @@ -40,6 +41,7 @@ type JointInferenceService struct { | |||
| type JointInferenceServiceSpec struct { | |||
| EdgeWorker EdgeWorker `json:"edgeWorker"` | |||
| CloudWorker CloudWorker `json:"cloudWorker"` | |||
| appsv1.DeploymentSpec `json:",inline"` | |||
There was a problem hiding this comment.
Why add a definition of DeploymentSpec to the api
There was a problem hiding this comment.
It's my oversight. The definition was not used in the code and I have removed it.
There was a problem hiding this comment.
There are CI issues that remains to fix, see https://github.com/kubeedge/sedna/pull/446/checks?check_run_id=31278018789
| corelisters "k8s.io/client-go/listers/apps/v1" | ||
| corelistersv1 "k8s.io/client-go/listers/core/v1" | ||
| "k8s.io/client-go/tools/record" | ||
| "k8s.io/client-go/util/workqueue" |
There was a problem hiding this comment.
Import into three groups (internal package/external package/project reference package)
for example:
context
...
v1 "k8s. IO/API/core/v1"
...
"github.com/kubeedge/sedna/pkg/globalmanager/config"
...
| deploymentInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ | ||
| AddFunc: jc.addDeployment, | ||
| UpdateFunc: jc.updateDeployment, | ||
| DeleteFunc: jc.deleteDeployment, |
There was a problem hiding this comment.
If the user manually deleted deployment, whether deployment will be rebuilt？
| deploymentStoreSynced cache.InformerSynced | ||
| // A store of deployment | ||
| deploymentsLister appslisters.DeploymentLister |
There was a problem hiding this comment.
Now that deployment is in use as a controller, can the pod related monitoring logic be removed from the code?
| return | ||
| } | ||
| for _, deployment := range deployments { | ||
| c.kubeClient.AppsV1().Deployments(curService.Namespace).Delete(context.TODO(), deployment.Name, metav1.DeleteOptions{}) |
There was a problem hiding this comment.
Build deployment first, and then use update function to update, rather than simply delete deployment
newDeployment := ...
c.kubeClient.AppsV1().Deployments(newDeployment.Namespace).Update(context.TODO(), newDeployment, metav1.UpdateOptions{})
| k8serrors "k8s.io/apimachinery/pkg/api/errors" | ||
| "k8s.io/apimachinery/pkg/api/resource" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/labels" | ||
| "k8s.io/apimachinery/pkg/watch" | ||
| kubernetesfake "k8s.io/client-go/kubernetes/fake" | ||
| "k8s.io/client-go/kubernetes/scheme" | ||
| v1core "k8s.io/client-go/kubernetes/typed/core/v1" | ||
| corelisters "k8s.io/client-go/listers/core/v1" | ||
| "k8s.io/client-go/tools/record" | ||
| "k8s.io/client-go/util/workqueue" |
SherlockShemol
force-pushed
the
fl-controller-enhancement
branch
6 times, most recently
from
be8aa11 to
82add2d
Compare
…ion of federated learning cotroller. \n 2.enable federated learning pod to rebuild itself if it is manually or wrongly deleted. \n 3.enable self updating pod config when modifying CRD of federated learning. \n 4.add test file to ensure the correctness of the solution. Signed-off-by: SherlockShemol <[email protected]>
SherlockShemol
force-pushed
the
fl-controller-enhancement
branch
from
c3953fa to
b37522e
Compare
SherlockShemol
changed the title
|
/lgtm |
|
/lgtm |
1 similar comment
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jaypume 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:
Approvers can indicate their approval by writing |
kubeedge-bot
added
the
approved
What type of PR is this?
What this PR does / why we need it:
FeratedLearningJob controller enhancement
Which issue(s) this PR fixes:
Fixes #