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

refactor(artifacts): Combine expected artifacts and trigger artifact constraints #6634

Merged
merged 32 commits into from
Mar 5, 2019

Conversation

jkschneider
Copy link
Member

@jkschneider jkschneider commented Mar 2, 2019

See #6613 for details about why and further screenshots.

Depends on spinnaker/clouddriver#3424

Jon Schneider and others added 18 commits February 25, 2019 11:23
* Extracted body of spelAutocompleteService.service.js into a separate class and converted to Typescript
* Tweaked CSS class of react-based expected artifact inputs that occur in K8S/AppEngine stage configurations to match the size of text seen elsewhere on those modals.
* ExpectedArtifactEditor now takes as a property input an IPipeline (optionally) that is in turn used to populate expression suggestions.
TODO:
- user is unable to edit or remove expected artifacts created with the StageArtifactSelector
…reincarnate

feat(artifacts): add a flag to hide the expected artifacts component
@jkschneider
Copy link
Member Author

This is the "Deploy Manifest" stage after incorporating the new selector:

image

It adds requiredArtifacts and matchArtifact to the existing matchArtifactId and requiredArtifactIds. These fields are of type IArtifact and are present when one of the bindable artifacts or the match artifact are defined inline in the stage.

image

@jkschneider jkschneider requested a review from a user March 2, 2019 22:57
@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@jkschneider jkschneider changed the title Combine artifacts triggers refactor(artifacts): Combine expected artifacts and trigger artifact constraints Mar 2, 2019
* Assume that clouddriver will provide Docker artifact credentials if a cloud provider is configured that can use these kinds of artifacts.
* Do not allow definition of a k8s artifact as a trigger artifact constraint.
* Do not allow definition of a docker artifact as an inline "Deploy Manifest" manifest artifact.
@jkschneider
Copy link
Member Author

Here is a screenshot of the new Kubernetes artifact editor:

image

@ghost
Copy link

ghost commented Mar 4, 2019

Looks like the unflagged experience is still a bit broken. In this screenshot the GCS editable input where I would enter the bucket's gs:// URL is missing from the inline editor:

screen shot 2019-03-04 at 11 34 57 am

Additionally looks as though adding an HTTP default artifact causes console errors with no input field showing up:

screen shot 2019-03-04 at 11 46 06 am

screen shot 2019-03-04 at 11 47 01 am

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I'm still checking my existing pipelines on this branch to find any more breakages but am adding these first comments from a quick code review.

@ghost
Copy link

ghost commented Mar 4, 2019

I think I see the issue with the http default artifact - it looks like it shouldn't be presented there at all; it doesn't appear on master.

@jkschneider
Copy link
Member Author

jkschneider commented Mar 4, 2019

I think I see the issue with the http default artifact - it looks like it shouldn't be presented there at all; it doesn't appear on master.

yes, i suppose we are going to have to add an angular default http editor :/

@jkschneider
Copy link
Member Author

@sbwsg I added a default HTTP editor so that works now:

image

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

hokay, code review looks good to me. I checked out the branch and ran through a few of my k8s and GAE pipelines that utilize artifacts (without the rewrite flag enabled). Clicking around and configuring the pipelines looks to be working as expected and running the pipelines continues to work just fine as well (I wasn't expecting any difference there but wanted to double check).

At this stage I think this should get merged. Let's keep an eye out for user reports of issues related to artifacts in the nightly builds and when 1.13 gets cut. I'm still a bit cautious that we might get reports of problems in artifact edge cases that we're either not aware of or don't exercise very often but I think we can deal with those if and when they surface.

@jkschneider jkschneider merged commit 5da2965 into spinnaker:master Mar 5, 2019
anotherchrisberry added a commit that referenced this pull request Mar 12, 2019
#6672)

* chore(core): Bump version to 0.0.343

eacfe75 feat(core): Warning message about invalid job params (#6669)
a28f7af fix(core): Surface invalid params for a pipeline stage (#6668)
e2b4d8e refactor(*): remove unused local storage caches (#6665)
35e82ac feat(core): Add support for an Artifactory Trigger (#6664)
bc2bf43 feat(jenkins): Add artifact status tab to Jenkins execution details (#6666)
e0bda86 feat(artifacts): Re-use artifacts when re-running a pipeline (#6663)
29582dd feat(core): Filter v2 pipeline templates from create pipeline modal (#6660)
45678a1 fix(core/pipeline): make cancelmodal take markdown for body (#6662)
02af17f fix(artifacts): Correct render-if-feature for new artifacts on stage 'produces artifact' (#6661)
0eb634c fix(artifacts): Maven/ivy reference field, Base64 validation, SpelText performance (#6656)
93e89be fix(core): Remove ability to trigger manual exec for mptv2 pipelines (#6651)
d3209d4 feat(core): Add pipeline to IStageConfigProps (#6655)
1bc94a1 chore(angularjs): explicitly annotate more angularjs injections (#6653)
78d0aae fix(artifacts): save HTTP URL as artifact reference (#6650)
09d7fee fix({core,amazon}/serverGroup): filter out empty tags, change 'tags' field type (#6645)
8e9cb0b fix(core): titus run jobs override all other providers (#6647)
12e3bff fix(core): Remove configure button and setup redirect for mptv2 pipeline (#6644)
5da2965 refactor(artifacts): Combine expected artifacts and trigger artifact constraints (#6634)
eae6b45 feat(users): Always surface authenticated user for executions/tasks (#6638)

* chore(amazon): Bump version to 0.0.179

e2b4d8e refactor(*): remove unused local storage caches (#6665)
09d7fee fix({core,amazon}/serverGroup): filter out empty tags, change 'tags' field type (#6645)

* chore(titus): Bump version to 0.0.79

e2b4d8e refactor(*): remove unused local storage caches (#6665)
maggieneterval added a commit to maggieneterval/deck that referenced this pull request May 10, 2019
1ada021 feat(kubernetes): expose rendered helm template in execution details (spinnaker#6943)
d122749 feat(runJob/kubernetes): render external link (spinnaker#6930)
9e03e52 refactor(runJob/kubernetes): refactor exec details (spinnaker#6924)
98ca0ff refactor(runJob/kubernetes): use joblogviewer (spinnaker#6917)
6c5eed6 feat(provider/kubernetes): run job manifest artifacts (spinnaker#6902)
cd8780c fix(kubernetes): fix discrepancy between orca deploy stage model and artifacts rewrite stage model
d44be0c fix(kubernetes): hide manifest artifact selector in text mode
bd94593 feat(kubernetes): remove rollout strategies feature flag
0ec6a7e feat(kubernetes): validate text manifests when rollout strategies enabled
37e24d7 fix(kubernetes): safer lookups for deploy stage validators (spinnaker#6847)
478e326 feat(kubernetes): add rollout strategies to deploy manifest stage (spinnaker#6841)
e618548 feat(preconfiguredJob): logs for k8s jobs (spinnaker#6840)
c30058a feat(k8s): Exclude inline base 64 artifact editing in k8s manifest (spinnaker#6839)
5640431 fix(k8s): Fix deploy manifest (spinnaker#6833)
ee89a64 feat(kuberntes): v2 runJob (spinnaker#6831)
2d7f388 feat(kubernetes): feature-flagged support for kubernetes traffic management strategies (spinnaker#6816)
33ad914 chore(kubernetes): refactor BasicSettings component to be usable in stages (spinnaker#6820)
72e164d refactor(core): de-angularize ApplicationModelBuilder, fix project executions (spinnaker#6802)
d7f6fbd fix(kubernetes): show Deployment clusters in Find Artifacts from Resource stages (spinnaker#6794)
20d0d7a fix(kubernetes): safe lookups for apiVersion on patch manifest deploy status (spinnaker#6775)
bf13a3c fix(kubernetes): fix validation for Find Artifact from Resource stage (spinnaker#6777)
50b74f2 feat(gcb): add Google Cloud Build stage (spinnaker#6774)
5b03078 fix(kubernetes): fix copy manifest from infrastructure button (spinnaker#6719)
57c30b5 fix(kubernetes): Use apiGroup when looking up deploy status for CRDs (spinnaker#6691)
a5a54bd feat(kubernetes): add expression evaluation options to bake and deploy manifest stages (spinnaker#6696)
51eeba4 chore(core): upgrade the version to formik 1.4.1 (spinnaker#6705)
e2b4d8e refactor(*): remove unused local storage caches (spinnaker#6665)
5da2965 refactor(artifacts): Combine expected artifacts and trigger artifact constraints (spinnaker#6634)
0d37cb2 fix(kubernetes): only set manifestName in static mode manifest selector
79a0700 feat(kubernetes): add dynamic target selection to patch manifest stage
0abce66 fix(kubernetes): remove unnecessary delete manifest stage defaults
5bf6045 fix(kubernetes): allow text input for replicas in `Scale (Manifest)` stage (spinnaker#6630)
cbfcae4 feat(kubernetes): add label mode to manifest selector component to enable dynamic target selection in delete manifest stage (spinnaker#6628)
d828a53 chore(angularjs): Explicitly annotate directive controllers
3e75815 refactor(core): migrate momentjs functionality to luxon + date-fns (spinnaker#6604)
7d5fc34 chore(prettier): Just Use Prettier™ (spinnaker#6600)
04bb4a0 fix(html): Fix various invalid HTML (spinnaker#6599)
5cf6c79 chore(prettier): Just Use Prettier™
3ffa4fb chore(angularjs): Do not use .component('foo', new Foo())
4b91b36 refactor(core): move Ace Editor CSS to core module (spinnaker#6588)
cc52bee chore(angularjs): Remove all 'ngInject'; in favor of explicit DI annotation
b6bab1e chore(prettier): Just Use Prettier™
f3fd790 chore(angularjs): Explicitly annotate all AngularJS injection points
629613f fix(kubernetes): properly detect if autoscaler is attached to server groups (spinnaker#6578)
d72bc17 fix(eslint): Fix eslint warnings for @typescript-eslint/camelcase
maggieneterval added a commit that referenced this pull request May 10, 2019
1ada021 feat(kubernetes): expose rendered helm template in execution details (#6943)
d122749 feat(runJob/kubernetes): render external link (#6930)
9e03e52 refactor(runJob/kubernetes): refactor exec details (#6924)
98ca0ff refactor(runJob/kubernetes): use joblogviewer (#6917)
6c5eed6 feat(provider/kubernetes): run job manifest artifacts (#6902)
cd8780c fix(kubernetes): fix discrepancy between orca deploy stage model and artifacts rewrite stage model
d44be0c fix(kubernetes): hide manifest artifact selector in text mode
bd94593 feat(kubernetes): remove rollout strategies feature flag
0ec6a7e feat(kubernetes): validate text manifests when rollout strategies enabled
37e24d7 fix(kubernetes): safer lookups for deploy stage validators (#6847)
478e326 feat(kubernetes): add rollout strategies to deploy manifest stage (#6841)
e618548 feat(preconfiguredJob): logs for k8s jobs (#6840)
c30058a feat(k8s): Exclude inline base 64 artifact editing in k8s manifest (#6839)
5640431 fix(k8s): Fix deploy manifest (#6833)
ee89a64 feat(kuberntes): v2 runJob (#6831)
2d7f388 feat(kubernetes): feature-flagged support for kubernetes traffic management strategies (#6816)
33ad914 chore(kubernetes): refactor BasicSettings component to be usable in stages (#6820)
72e164d refactor(core): de-angularize ApplicationModelBuilder, fix project executions (#6802)
d7f6fbd fix(kubernetes): show Deployment clusters in Find Artifacts from Resource stages (#6794)
20d0d7a fix(kubernetes): safe lookups for apiVersion on patch manifest deploy status (#6775)
bf13a3c fix(kubernetes): fix validation for Find Artifact from Resource stage (#6777)
50b74f2 feat(gcb): add Google Cloud Build stage (#6774)
5b03078 fix(kubernetes): fix copy manifest from infrastructure button (#6719)
57c30b5 fix(kubernetes): Use apiGroup when looking up deploy status for CRDs (#6691)
a5a54bd feat(kubernetes): add expression evaluation options to bake and deploy manifest stages (#6696)
51eeba4 chore(core): upgrade the version to formik 1.4.1 (#6705)
e2b4d8e refactor(*): remove unused local storage caches (#6665)
5da2965 refactor(artifacts): Combine expected artifacts and trigger artifact constraints (#6634)
0d37cb2 fix(kubernetes): only set manifestName in static mode manifest selector
79a0700 feat(kubernetes): add dynamic target selection to patch manifest stage
0abce66 fix(kubernetes): remove unnecessary delete manifest stage defaults
5bf6045 fix(kubernetes): allow text input for replicas in `Scale (Manifest)` stage (#6630)
cbfcae4 feat(kubernetes): add label mode to manifest selector component to enable dynamic target selection in delete manifest stage (#6628)
d828a53 chore(angularjs): Explicitly annotate directive controllers
3e75815 refactor(core): migrate momentjs functionality to luxon + date-fns (#6604)
7d5fc34 chore(prettier): Just Use Prettier™ (#6600)
04bb4a0 fix(html): Fix various invalid HTML (#6599)
5cf6c79 chore(prettier): Just Use Prettier™
3ffa4fb chore(angularjs): Do not use .component('foo', new Foo())
4b91b36 refactor(core): move Ace Editor CSS to core module (#6588)
cc52bee chore(angularjs): Remove all 'ngInject'; in favor of explicit DI annotation
b6bab1e chore(prettier): Just Use Prettier™
f3fd790 chore(angularjs): Explicitly annotate all AngularJS injection points
629613f fix(kubernetes): properly detect if autoscaler is attached to server groups (#6578)
d72bc17 fix(eslint): Fix eslint warnings for @typescript-eslint/camelcase
@Jammy-Louie Jammy-Louie deleted the combine-artifacts-triggers branch May 21, 2019 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants