test-infra

test-infra copied to clipboard

At the moment only .yaml and .yml file extensions are allowed for prow job configs... let's go ahead and allow .json files also.

Hi @midnightconman. 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.

@midnightconman is that the only change needed? At least some unit tests could verify that assumption...

@midnightconman is that the only change needed? At least some unit tests could verify that assumption...

I looked around and didn't see any existing unit tests for the .yaml or .yml cases, can you please point me at where those are or where these new ones should go?

Sorry if I wasn't clear in my previous comment... I meant that yamlToConfig should probably be modified to understand JSON files as well, and that part needs tests. There is also the documentation part that needs to be addressed.

I meant that yamlToConfig should probably be modified to understand JSON files as well, and that part needs tests.

The yaml library used by config 100% supports json input:

In short, this library first converts YAML to JSON using go-yaml and then uses json.Marshal and json.Unmarshal to convert to or from the struct. This means that it effectively reuses the JSON struct tags as well as the custom JSON methods MarshalJSON and UnmarshalJSON unlike go-yaml. For a detailed overview of the rationale behind this method, [see this blog post](http://web.archive.org/web/20190603050330/http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/).

Since this lib supports json, I think json support is implied and a requirement. If this lib was changed to not support json, many other places in the code base would need to be changed.

We (and kubernetes/test-infra) have been using json for job and prow configs for years... the prow job config loader just doesn't seek out json files (because they are not popular in ks8/test-infra). This PR changes that to also include json files when seeking out prow jobs by directory.

There is also the documentation part that needs to be addressed.

I don't think this feature is documented... let me look around and see where would be a good place to call out the prow config directory inclusion logic. 😸

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

/remove-lifecycle rotten

/ok-to-test

Thank you @petr-muller , is this good to merge?

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: midnightconman, petr-muller

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:

• ~~prow/OWNERS~~ [petr-muller]

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

/hold

As I pushed the approve button, I realized that this is maybe more dangerous than it looks - can't we break whoever has any *.json files alongside their job config (for whatever reason)? We'd try to parse them as prow job config and fail if we cannot. Would this warrant at least an announcement note?

Would this warrant at least an announcement note?

Sure, let me update ANNOUNCEMENTS.md.

New changes are detected. LGTM label has been removed.

Would this warrant at least an announcement note?

Sure, let me update ANNOUNCEMENTS.md.

Ohh, looks like the announcements file was deprecated... how should we call out this change?

Ohh, looks like the announcements file was deprecated... how should we call out this change?

I think the new location is https://github.com/kubernetes-sigs/prow/blob/main/site/content/en/docs/ANNOUNCEMENTS.md which backs https://docs.prow.k8s.io/docs/announcements/

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle rotten
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/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.