Extract pingsource-jobrunner deployment into manifest #804
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aliok 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 |
openshift-ci-robot
added
the
approved
|
the 0.14.x matches the 1.8.0 product I guess we than have to carry this patch on a few more branches? |
|
@matzew Let's focus only on 0.14 for now which is to be in the 1.8.0 that's to be released soon. |
|
Note that, after this is merged:
|
aliok
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
| @@ -0,0 +1 @@ | |||
| core/deployments/pingsource-jobrunner.yaml | |||
There was a problem hiding this comment.
Symlink to new deployment yaml file
| - name: JOB_RUNNER_IMAGE | ||
| value: ko://knative.dev/eventing/cmd/ping/jobrunner |
There was a problem hiding this comment.
This thing is now moved to the manifest and created by the operator.
No need to pass this around.
| @@ -0,0 +1,70 @@ | |||
| # Copyright 2018 The Knative Authors | |||
There was a problem hiding this comment.
New deployment for the pingsource jobrunner. This is to be included in the manifest.
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| apiVersion: apps/v1 |
There was a problem hiding this comment.
Values here is taken from the programmatic deployment creation here: https://github.com/knative/eventing/blob/f8b42c3b6e13097bc53283076a686f4fdb50ab52/pkg/reconciler/pingsource/controller/resources/jobrunner.go#L43
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: pingsource-jobrunner | ||
| namespace: knative-eventing |
There was a problem hiding this comment.
Not sure if I should touch should file manually...
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: pingsource-jobrunner | ||
| namespace: knative-eventing | ||
| labels: | ||
| eventing.knative.dev/release: devel | ||
| spec: | ||
| replicas: 0 |
There was a problem hiding this comment.
Not sure if I should touch this file or if this stuff will be generated.
| return nil, fmt.Errorf("error getting job runner deployment %v", err) | ||
| } else if podSpecChanged(d.Spec.Template.Spec, expected.Spec.Template.Spec) { | ||
| d.Spec.Template.Spec = expected.Spec.Template.Spec | ||
| } else if *d.Spec.Replicas != 1 { |
There was a problem hiding this comment.
Prev approach: create the deployment for the job runner
Now: scale up the already created deployment
|
@aliok: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/close Won't fix this problem for this version. Will be fixed in 1.10 (upstream 1.16) |
|
@aliok: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Upstream issue: knative#3035
JIRA: https://issues.redhat.com/browse/SRVKE-473
I don't think it is a good idea to merge this back to upstream 0.14, which was released on May 1st.
Let's fix it here and then I can show this PR to the community.