graph-builder: move plugins to cincinnati crate and catalog, and make plugins configurable #234

steveej · 2020-02-27T20:08:34Z

This also reorganizes the imports, common code to all plugins, and
adjusts the terminology.

Fixes #237.

vrutkovs · 2020-02-28T12:00:05Z

Requires openshift/release#7370 to pass (secret change)

steveej · 2020-03-02T15:27:48Z

@vrutkovs do you know why this happened in the cargo-test job?

 2020/02/29 01:16:33 error: unable to signal to artifacts container to terminate in pod cargo-test, triggering deletion: could not run remote command: unable to upgrade connection: container not found ("artifacts")
2020/02/29 01:16:33 error: unable to retrieve artifacts from pod cargo-test: could not read gzipped artifacts: unable to upgrade connection: container not found ("artifacts")
{"component":"entrypoint","file":"prow/entrypoint/run.g

I do expect the test itself to fail right now, but this doesn't look like the failure I expected.

vrutkovs · 2020-03-02T15:38:33Z

/retest

Seems to be yet another CI flake

steveej · 2020-03-03T08:37:30Z

/retest

steveej · 2020-03-03T19:52:47Z

/test ci/prow/cargo-test

Triggering a re-test as CI should be fixed after openshift/release#7449.

steveej · 2020-03-03T19:56:55Z

Trying again.

/test cargo-test

vrutkovs · 2020-03-04T09:11:42Z

test_get_labels wants CINCINNATI_TEST_QUAY_API_TOKEN

steveej · 2020-03-04T09:30:00Z

test_get_labels wants CINCINNATI_TEST_QUAY_API_TOKEN

The secret exists since there is no error around after reading them at https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_cincinnati/234/pull-ci-openshift-cincinnati-master-cargo-test/974#1:build-log.txt%3A15, but it's decipherable by the test:

thread 'net::private::test_wrong_auth' panicked at 'CINCINNATI_TEST_QUAY_API_TOKEN missing: NotUnicode("\xA9\xF85\u{14}

steveej · 2020-03-04T09:56:18Z

/retest

steveej · 2020-03-04T09:58:48Z

/test cargo-test

vrutkovs

Few minor notes, looks good otherwise

...ati/src/plugins/internal/graph_builder/github_openshift_secondary_metadata_scraper/plugin.rs

cincinnati/src/plugins/internal/graph_builder/commons.rs

cincinnati/src/plugins/internal/graph_builder/release.rs

vrutkovs · 2020-03-04T10:34:11Z

graph-builder/src/config/file.rs

There are currently no requirements for Cincinnati releases (there aren't any really :) ). Do we want to keep it? I think its safe to remove it now, while we control the template and no disconnected installs are in the wild

I will be very happy to remove it ASAP, but I'm hesitant to add the change to this PR. How about a separate ticket for rethinking the options?

Do we have more deprecated options to remove though? I don't mind removing these later

I would like to remove all UpstreamOptions but I didn't want to touch all of them in this PR. I touched this one now because it's not in the correct place. I added this option to the ServiceOptions.

hack/e2e.sh

steveej · 2020-03-04T11:13:27Z

/retest

steveej · 2020-03-04T11:13:49Z

/test cargo-test

…configurable The primary goal of this change is to make the graph-builder plugins configurable via a config file. This required all of the configurable plugins to be in the plugin catalog, which lives in the cincinnati crate. Therefore all graph-builder plugins are moved to the cincinnati crate, leaving the graph-builder crate with only its core functionality. Subsequently the graph-builder is made configurable, with a few refactors along the way as they fit in naturally with this change: * and the naming of the related functions and variable names in the graph-builder and policy-engine are normalized. * the yaml_lint

…_path This tolerates the empty string credentials_path when deserializing the the plugin settings. This is to alleviate some difficulties with deployment tools, where it is sometimes easier to set an empty than avoiding to set it.

This moves the `pause-secs` setting from the upstream.registry to the service options, because the pause occurs in the core service loop, and is not specific to any upstream method or plugin.

…usize This makes the fetch_concurrency setting an integer in the TOML configuration files instead of a String. This seems more intuitive to me from a user perspective, as I wouldn't surround an integer number with quotes intuitively.

* Rename the secret to a recent secret rename. * Create dummy secret in e2e job as the real secrets are unavailable here.

steveej · 2020-03-04T12:47:20Z

/test cargo-test

vrutkovs

/lgtm

vrutkovs

/lgtm

openshift-ci-robot · 2020-03-04T15:49:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: steveeJ, vrutkovs

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~~ [steveeJ,vrutkovs]

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

steveej requested review from LalatenduMohanty and vrutkovs February 27, 2020 20:08

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 27, 2020

openshift-ci-robot requested review from lucab and pbergene February 27, 2020 20:08

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 27, 2020

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch from 9c90cd4 to 870daa9 Compare February 27, 2020 20:19

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 27, 2020

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch 4 times, most recently from 295487b to bfe13cd Compare February 27, 2020 21:01

steveej mentioned this pull request Feb 27, 2020

graph-builder: use openshift/cincinnati-graph-data instead of quay labels #232

Merged

6 tasks

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch from bfe13cd to 20154d4 Compare February 28, 2020 21:09

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 28, 2020

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch from 20154d4 to c6ef825 Compare February 28, 2020 21:14

steveej removed request for lucab and pbergene March 2, 2020 15:25

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch 2 times, most recently from b7f91d9 to 820a9a4 Compare March 2, 2020 16:32

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch 3 times, most recently from 72b8021 to e90cf44 Compare March 3, 2020 15:34

vrutkovs suggested changes Mar 4, 2020

View reviewed changes

steveej added 6 commits March 4, 2020 12:24

graph-builder: move 'pause-secs' to service options

730862c

This moves the `pause-secs` setting from the upstream.registry to the service options, because the pause occurs in the core service loop, and is not specific to any upstream method or plugin.

e2e,dist/openshift: adapt to secret name changes and CI limitations

cc679c2

* Rename the secret to a recent secret rename. * Create dummy secret in e2e job as the real secrets are unavailable here.

e2e,dist/openshift: make credentials_path optional; don't pass it in e2e

a818db1

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch from 21497b9 to a818db1 Compare March 4, 2020 11:24

vrutkovs approved these changes Mar 4, 2020

View reviewed changes

openshift-ci-robot assigned vrutkovs Mar 4, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 4, 2020

disable all quay private tests due to CI issues

0395dc0

steveej force-pushed the pr/graph-builder-make-plugins-configurable branch from bdfba2c to 0395dc0 Compare March 4, 2020 15:00

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 4, 2020

vrutkovs approved these changes Mar 4, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 4, 2020

openshift-merge-robot merged commit 0ea6690 into openshift:master Mar 4, 2020

steveej mentioned this pull request Mar 6, 2020

Safely handle the case where no credentials file is provided #238

Closed

steveej deleted the pr/graph-builder-make-plugins-configurable branch September 25, 2020 12:52

graph-builder: move plugins to cincinnati crate and catalog, and make plugins configurable #234

graph-builder: move plugins to cincinnati crate and catalog, and make plugins configurable #234

Uh oh!

Conversation

steveej commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vrutkovs commented Feb 28, 2020

Uh oh!

steveej commented Mar 2, 2020

Uh oh!

vrutkovs commented Mar 2, 2020

Uh oh!

steveej commented Mar 3, 2020

Uh oh!

steveej commented Mar 3, 2020

Uh oh!

steveej commented Mar 3, 2020

Uh oh!

vrutkovs commented Mar 4, 2020

Uh oh!

steveej commented Mar 4, 2020

Uh oh!

steveej commented Mar 4, 2020

Uh oh!

steveej commented Mar 4, 2020

Uh oh!

vrutkovs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vrutkovs Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

steveej Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

vrutkovs Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

steveej Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

steveej commented Mar 4, 2020

Uh oh!

steveej commented Mar 4, 2020

Uh oh!

steveej commented Mar 4, 2020

Uh oh!

vrutkovs left a comment

Choose a reason for hiding this comment

Uh oh!

vrutkovs left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Mar 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

steveej commented Feb 27, 2020 •

edited

Loading