Skip to content

Conversation

@bbguimaraes
Copy link
Contributor

Slightly modified subset of #85.

Open questions:

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 20, 2019
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 20, 2019
@stevekuznetsov
Copy link
Contributor

As a first pass, we should be concerned with identifying the set of information necessary to work with downstream/at runtime. Let's validate that every field exists, resources are set, etc. No concept of name resolution yet

@bbguimaraes
Copy link
Contributor Author

Another one:

  • We need a way to reference an image that is not part of the pipeline,

Container tests don't have that problem because they simply don't allow it, templates use the IMAGE_/LOCAL_IMAGE_ prefixes.


type TestStep struct {
Name string `json:"name,omitempty"`
Image string `json:"image,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a string and not a PipelineImageReference don't we already allow any pull spec here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two points in the execution where this is used:

  • When configuration is loaded, to determine dependencies between (ci-operator) steps.
  • When the pods are created.

The biggest problem is #1. We know at that point the images that are part of the pipeline, but not those that come from tag_specification.

For #2, we can always use pipeline:src, stable:tests, or even a full spec, which is what I've been using, but since #1 requires us to have that information, that should not be required.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we generally operate on the concept that if an image is named foo and there exists a foo in the images brought in by the tag_specification, the built image overrides that which is pulled in. So, we can accept a simple string here and:

  • if it matches something that is built by the pipeline or the images, create that link in the steps
  • if it does not, expect it to be provided by the import from tag-specification

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I wanted to hear/read.

@bbguimaraes bbguimaraes changed the title WIP ci-operator: add TestStep type ci-operator: add TestStep type Aug 20, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 20, 2019
Copy link
Contributor

@stevekuznetsov stevekuznetsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 20, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bbguimaraes, stevekuznetsov

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [bbguimaraes,stevekuznetsov]

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

@openshift-merge-robot openshift-merge-robot merged commit a77c2d3 into openshift:master Aug 20, 2019
@bbguimaraes bbguimaraes deleted the steps_api branch August 20, 2019 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants