Make OnFailure as default restartPolicy for BroadcastJob

[Shubhamurkade]

Ⅰ. Describe what this PR does

Creation of a BroadcastJob without specifying restartPolicy fails with the following error:

Error from server: error when creating "broadcastjob_kruise.yaml": admission webhook "vbroadcastjob.kb.io" denied the request: spec.template.spec.restartPolicy: Invalid value: "Always": pod restartPolicy can only be Never or OnFailure

Root Cause:
restartPolicy is present under spec.template.spec block. This block is defined with PodSpec structure and the restartPolicy element in it has a default value of Always.

Solution:
For BroadcastJob changed the default value to OnFailure in /Users/surkade/Documents/dev/kruise/pkg/webhook/broadcastjob/validating/broadcastjob_create_update_handler.go file

Ⅱ. Does this pull request fix one issue?

No

Ⅲ. Describe how to verify it

Create a sample BroadcastJob with the following template (Do not specify restartPolicy under spec.template.spec:

apiVersion: apps.kruise.io/v1alpha1
kind: BroadcastJob
metadata:
    labels:
        app: <name>
    name: <name>
spec:
    template:
        metadata:
            labels:
                app: <name>
        spec:
            containers:
            - image: <image>
              imagePullPolicy: <policy>
              name: <name>
              ports:
              - containerPort: <port>

The previously mentioned error can be seen

Ⅳ. Special notes for reviews

I have kept the new default for restartPolicy as OnFailure. Please let me know if Never would make more sense as a default.

[kruise-bot]

Welcome @Shubhamurkade! It looks like this is your first PR to openkruise/kruise 🎉

[veophi]

@Shubhamurkade Maybe, you should modify this function instead of event handler :)

kruise/apis/apps/defaults/v1alpha1.go

Line 178 in 1ece7fe

func SetDefaultsBroadcastJob(obj *v1alpha1.BroadcastJob, injectTemplateDefaults bool) {

.

[Shubhamurkade]

@Shubhamurkade Maybe, you should modify this function instead of event handler :)

kruise/apis/apps/defaults/v1alpha1.go

Line 178 in 1ece7fe

func SetDefaultsBroadcastJob(obj *v1alpha1.BroadcastJob, injectTemplateDefaults bool) {

.

@veophi Thanks for the suggestion. Will do!

[Shubhamurkade]

@veophi Please let me know if the latest changes look fine.

[zmberg]

@Shubhamurkade It's good, and you should git commit -s and push again.

[codecov-commenter]

Codecov Report

Merging #1149 (83049b8) into master (53fb7e1) will increase coverage by 0.31%.
The diff coverage is 16.08%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #1149      +/-   ##
==========================================
+ Coverage   49.73%   50.04%   +0.31%     
==========================================
  Files         137      143       +6     
  Lines       19331    19843     +512     
==========================================
+ Hits         9614     9931     +317     
- Misses       8667     8826     +159     
- Partials     1050     1086      +36     

Flag	Coverage Δ
unittests	50.04% <16.08%> (+0.31%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/control/pubcontrol/pub_control.go	25.00% <0.00%> (-1.22%)	⬇️
pkg/control/pubcontrol/pub_control_utils.go	50.99% <0.00%> (-1.04%)	⬇️
...tentpodstate/persistent_pod_state_event_handler.go	0.00% <0.00%> (ø)
...availablebudget/podunavailablebudget_controller.go	45.48% <0.00%> (+6.26%)	⬆️
...ller/podunavailablebudget/pub_pod_event_handler.go	26.78% <0.00%> (+0.31%)	⬆️
...g/webhook/pod/validating/pod_unavailable_budget.go	68.83% <ø> (+5.73%)	⬆️
...sistentpodstate/persistent_pod_state_controller.go	31.18% <18.18%> (-1.68%)	⬇️
pkg/webhook/pod/mutating/persistent_pod_state.go	64.21% <20.00%> (-1.73%)	⬇️
...ate/validating/persistent_create_update_handler.go	59.77% <71.42%> (+1.23%)	⬆️
...ablebudget/validating/pub_create_update_handler.go	62.09% <100.00%> (+1.59%)	⬆️
... and 31 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[zmberg]

@Shubhamurkade You need git commit -s and squash the commits to one.

[zmberg]

@Shubhamurkade tks, and you must resolve the checks. https://github.com/openkruise/kruise/pull/1149/checks?check_run_id=11663449687

[Shubhamurkade]

@Shubhamurkade tks, and you must resolve the checks. https://github.com/openkruise/kruise/pull/1149/checks?check_run_id=11663449687

Please check now.

[furykerry]

/lgtm

[veophi]

/approve

[zmberg]

/approve

[kruise-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: veophi, zmberg

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 [zmberg]

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