Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USABILITY: Add cyclical pipeline references to validation errors #3053

Closed
jghiloni opened this issue Jan 15, 2019 · 11 comments

Comments

@jghiloni
Copy link
Contributor

commented Jan 15, 2019

Bug Report

If you have a cyclical reference in a job, for example:

jobs:
- name: my-job
  plan:
  - get: some-resource
    trigger: true
    passed: [my-job]

You can set the pipeline via fly but the UI ... is unhappy

Steps to Reproduce

Create a pipeline with a job that conforms to the passed criteria seen above

Expected Results

fly set-pipeline should error and point out the cyclical reference

Actual Results

fly set-pipeline works but the ATC doesn't render unless you remove the pipeline or log in as a user that doesn't have access to that pipeline

Version Info

  • Concourse version: 4.2.1
  • Deployment type (BOSH/Docker/binary): any
  • Infrastructure/IaaS: any
  • Browser (if applicable): chrome (probably any)
  • Did this used to work? shouldn't have!
@jghiloni

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2019

It should be noted that the /hd endpoint works

@vito

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

Technically this works and does something kind of interesting, though it's unhelpful that the UI detonates.

It means "run with whatever version I last succeeded with". Which is useful if you want to pin something and are too lazy to go see what it is.

...ok, we should probably just forbid it.

@vito

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

Prior discussion: concourse/atc#246

@jghiloni

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2019

What happens on the first run, in that case?

@vito

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

@jghiloni True, this only works if the job has run successfully at least once.

@DenverOps

This comment has been minimized.

Copy link

commented Jan 15, 2019

I have a pipeline.yml you can reference with a loop if you need it for testing.

@DonBower

This comment has been minimized.

Copy link

commented Jan 17, 2019

it should be noted that this causes the UI to fail as well:

jobs:
- name: my-job-1
  plan:
  - get: some-resource
    trigger: true
    passed: [my-job-3]
- name: my-job-2
  plan:
  - get: some-resource
    trigger: true
    passed: [my-job-1]
- name: my-job-3
  plan:
  - get: some-resource
    trigger: true
    passed: [my-job-2]

Point being that a "loop" can span more than one or two jobs.

@vito vito added the help wanted label Jan 17, 2019

@DenverOps

This comment has been minimized.

Copy link

commented Jan 17, 2019

Don and I can both help if needed.

@crsimmons

This comment has been minimized.

Copy link

commented Mar 14, 2019

We hit this issue today. The UI dies even if the pipeline is paused. We thought our Concourses/network had just died...

Maybe if there's a use case for having self-referencing passed constraints fly set-pipeline could at least warn you that something is afoot when it detects a cycle.

@cirocosta

This comment has been minimized.

Copy link
Member

commented Mar 14, 2019

Thanks for all of your input!

IIRC this has already been addressed in a slightly different way - not through a validation, but preventing the infinite loop that used to crash the UI and displaying the loop:

#3433 (review)

It should be there in 5.1.

I'll close this, for now, please let me know if I understood it incorrectly!

Thanks

@cirocosta cirocosta closed this Mar 14, 2019

@DenverOps

This comment has been minimized.

Copy link

commented Mar 14, 2019

@DonBower ^^^ looks like we have some movement we should looks at reviewing 5.1 when it drops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.