WIP: config: allow init containers if not decorated

[zhouhaibing089]

When the decorate is set as false, I think we can allow init containers
to be used. This is useful if we want to run prow where we can't rely on
gcs. We would still be able to configure our own containers to pull code
and ship logs, etc.

[k8s-ci-robot]

Hi @zhouhaibing089. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zhouhaibing089
To complete the pull request process, please assign cjwagner
You can assign the PR to them by writing /assign @cjwagner in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• prow/OWNERS

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

[zhouhaibing089]

Mark it as WIP as I haven't tested it yet.

@stevekuznetsov @fejta: Could you share some thoughts on this change?

[stevekuznetsov]

We would still be able to configure our own containers to pull code and ship logs, etc.

Why is this preferable to changing Prow in order to make it more featureful? I don't necessarily like this change if it means that users will work around Prow to deliver the same features Prow already does. What happens when you end up debugging the differences in how two different code checkout procedures run and bugs come out of having different implementations? That sounds like a headache.

[zhouhaibing089]

@stevekuznetsov: Thanks for sharing the thoughts here.

I don't necessarily like this change if it means that users will work around Prow to deliver the same features Prow already does.

Since decorate is false by default, it means prow doesn't really provide the same features generally. All the useful features(cloning code and shipping logs) are tied with GCS as far as I can see. The next steps I would like to work on is trying to make it working with other storage providers.

What happens when you end up debugging the differences in how two different code checkout procedures run and bugs come out of having different implementations?

I won't change the current behavior here, if the job is decorated, no other init containers would be allowed. If decorated is false, it means there is nothing to do with the git tools provided by prow. I personally feel it should not be a pain as it is clear where to look at depending on whether decorate is set to true or false.

[stevekuznetsov]

Cloning code has nothing to do with GCS. Uploading logs of course has to target a storage backend. I am suggesting that instead of letting someone choose to add their own custom implementation by turning off the decoration, let's let them choose which decoration they want. The work to write an uploader that targets a different cloud backend is the same, right?

[zhouhaibing089]

@stevekuznetsov: Today, there are four decorations which are being used:

1. clonerefs
2. initupload
3. entrypoint
4. sidecar

So your preference is to allow users to be able to choose the decorations they want like using cloneRefs but not others? I'd love to hear more clarifications on it.

[zhouhaibing089]

The work to write an uploader that targets a different cloud backend is the same, right?

That's something I'd like to do anyway. I'm just worried about whether the single decoration option is future-proof. Ideally, my preference is to let people choose the abstraction like:

presubmits:
  foo/bar:
    name: pull-bar-build
    decorations:
    - name: clonerefs
      params:
        arg1: val1
    - name: shiplogs
      params:
        arg1: val1

[stevekuznetsov]

I would turn the question to you, instead. What are you trying to do that Prow does not support today? As I take it, you do not use GCS. In my mind that implies one of the following:

• you do not want to upload any logs (tracked somewhere but I can't find it)
• you do want to upload logs, but use a different storage system (testgrid should support non-GCS stores for test results #7960)

I think both are valid. It may make sense for someone to run jobs that don't upload anything anywhere.

I do not want a solution where the job uploads artifacts to a different provider by turning off uploading and injecting something custom. I agree that configuring how uploads happen (and where they go) should follow the current configuration workflow -- there is a global default, defaults for orgs and repos, and then job-level fields to override.

[zhouhaibing089]

@stevekuznetsov: Just list few:

1. We have a repository where we can build kubernetes cluster from, any pull requests on that repository simply trigger a job that creates a custom resource, and then a controller will react on it to create the cluster for us to run e2e tests. The real git clone happens when the node is provisioned and it actually runs from the node, not within the job itself.

2. We also have a use case where we would like to look at the linked pull requests from the PR description, for example, I have a pull request on repository A, but I would like to build it together with another pull request in repository B. This happens because we have a dedicated repository for e2e test code, as a result, I may need to choose the test code when I've made some changes to my controller code.

I do not want a solution where the job uploads artifacts to a different provider by turning off uploading and injecting something custom.

Yeah, I agree. That's not my intention, either. If I can use decoration with another storage provider, that would be the right way to go.

[zhouhaibing089]

@stevekuznetsov: Kindly ping.. What is your opinion on the cases I mentioned above where I may need a customized init containers?

[stevekuznetsov]

Sorry, missed this one! For your first use-case you could set skip_cloning: true, right?

[stevekuznetsov]

For the second one, I am not sure we have a good solution, and that is kind of on purpose. In general I think we have found that trying to synchronize merges and tests between repositories is really error-prone and hard to work with. We suggest instead an approach where you add your tests (for instance) in a way that they can detect whether the new feature is turned on or not and then run as necessary. This is the same as other components -- when building and deploying decoupled components in a distributed system, make them all tolerant of some version skew to allow for seamless upgrades.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[k8s-ci-robot]

@zhouhaibing089: PR needs rebase.

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.

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.