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

[CI] Build and publish storybooks #87701

Merged
merged 45 commits into from
Feb 18, 2021
Merged

Conversation

brianseeders
Copy link
Contributor

@brianseeders brianseeders commented Jan 7, 2021

Closes #58024

  • Terraform-managed GCP resources
    • Public-facing GCS bucket, ci-artifacts.kibana.dev custom domain and SSL cert, service account and private key generation
    • The code for this is stored in a separate repository
    • Uploaded files are deleted automatically after 30 days
    • Bucket could be used for any publicly-accessible CI artifacts
  • Adds a ci_composite storybook that automatically includes all of the other storybooks (even if new ones get added)
  • During CI:
    • Create a github commit status for storybook building (both PRs and tracked branches, currently)
    • In one task, build all storybooks, including the composite one
    • Create index file that links to each storybook
    • gzip and upload files to a GCS bucket with a location that's unique for this commit
    • Upload the index page to the GCS bucket, at <branch>/latest/index.html, mainly so that tracked branches have a single URL that's always up-to-date
    • Complete the github commit status, with a link to the storybook index page
    • Add a link to the PR comment for the index page

I'm focused mostly on getting the storybooks building and uploading somewhere in this PR. I expect people who know more about Storybooks will make further improvements (e.g. to the composite) in separate PRs.

Example index page: https://ci-artifacts.kibana.dev/storybooks/pr-87701/6c5dca53b671f41da26d6d73ea536e4f8f2de511/index.html

Link added to the PR comment:

Screen Shot 2021-01-11 at 4 32 50 PM

which leads to a page that looks like this:

Screen Shot 2021-01-11 at 4 32 57 PM

which links to each individual Storybook, as well as the composite. The link is unique for this commit.

And here's what the checks/statuses look like:

Screen Shot 2021-01-11 at 5 44 46 PM

Screen Shot 2021-01-11 at 5 15 19 PM

The details link on the finished status links directly to the index page.

@brianseeders brianseeders added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:CI Continuous integration labels Jan 7, 2021
@brianseeders brianseeders changed the title [CI] Storybooks [CI] Build Storybooks and upload for public access Jan 11, 2021
@brianseeders brianseeders changed the title [CI] Build Storybooks and upload for public access [CI] Build and publish storybooks Jan 11, 2021

for(const alias of Object.keys(aliases).filter(a => a !== 'ci_composite')) {
// snake_case -> Title Case
const title = alias
Copy link
Contributor

Choose a reason for hiding this comment

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

In a draft PR (#80061) I started by making the aliases an object so that a title could be added. Doing this transformation would probably be fine if there were no title specified, but it would be better to allow a title. For instance here "apm" would end up as "Apm" where we want APM.

<p><a href="${getUrlForCommit()}/composite">Composite Storybook</a></p>
<h2>All</h2>
<ul>
${listHtml}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this page? What is the composite storybook missing that this gives us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want the links to the individual storybooks to be effectively hidden, so I built this index page to put in front of all of them. This seems particularly useful if you're only concerned about a particular storybook, because the composite takes a long time to load.

@smith
Copy link
Contributor

smith commented Jan 11, 2021

Thanks for kicking this off BTW!

@brianseeders
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM from an operations perspective

.collect { it.replace('/', '') }
.findAll { it != 'composite' }

def listHtml = storybooks.collect { """<li><a href="${getUrlForCommit()}/${it}">${it}</a></li>""" }.join("\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a tiny bit concerned about creating HTML with string concatenation, though our sources are pretty predictable it would be great if we could limit the character set to /[a-z\-_]/i or something

Comment on lines +191 to 192
${storybooksMessage}
${getDocsChangesLink()}
Copy link
Contributor

Choose a reason for hiding this comment

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

The newlines that end up in the list here cause the list to render much larger, which would be ideal to work around:

image

vs

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will get this fixed. I didn't notice before because it only happens if it's not the last link in the list (e.g. when there's a flaky suite)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, it's actually getDocsChangesLink() that's inserting a blank newline when it's missing. I can still get it fixed in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, my bad, makes sense. Thank you

@brianseeders
Copy link
Contributor Author

@elasticmachine merge upstream

@brianseeders
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

LGTM!

@brianseeders
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@brianseeders brianseeders added v7.12.0 v7.13.0 v7.11.2 auto-backport Deprecated - use backport:version if exact versions are needed labels Feb 18, 2021
@brianseeders brianseeders merged commit 03206b6 into elastic:master Feb 18, 2021
@brianseeders brianseeders deleted the storybooks-ci branch February 18, 2021 19:13
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 18, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 18, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

❌ 7.11: Commit could not be cherrypicked due to conflicts
7.12 / #91916
7.x / #91917

Successful backport PRs will be merged automatically after passing CI.

To backport manually, check out the target branch and run:
node scripts/backport --pr 87701

brianseeders added a commit to brianseeders/kibana that referenced this pull request Feb 18, 2021
# Conflicts:
#	src/dev/storybook/aliases.ts
kibanamachine added a commit that referenced this pull request Feb 18, 2021
kibanamachine added a commit that referenced this pull request Feb 18, 2021
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 19, 2021
* master: (111 commits)
  [Logs UI] Replace dependencies in the infra bundle (elastic#91503)
  [Search Source] Do not request unmapped fields if source filters are provided (elastic#91921)
  [APM] Kql Search Bar suggests values outside the selected time range (elastic#91918)
  Refactored component edit policy tests into separate folders and using client integration testing setup (elastic#91657)
  [Fleet] Don't error on missing package_assets value (elastic#91744)
  [Lens] Pass used histogram interval to chart (elastic#91370)
  [Indexpattern management] Use indexPatterns Service instead of savedObjects client (elastic#91839)
  [Security Solutions] Fixes Cypress tests for indicator match by making the selectors more specific (elastic#91947)
  [CI] backportrc can skip CI (elastic#91886)
  Revert "[SOM] fix flaky suites (elastic#91809)"
  [Fleet] Install Elastic Agent integration by default during setup (elastic#91676)
  [Fleet] Silently swallow 404 errors when deleting ingest pipelines (elastic#91778)
  [data.search] Use incrementCounter for search telemetry (elastic#91230)
  [Fleet] Bootstrap functional test suite (elastic#91898)
  [Alerts][Docs] Added API documentation for alerts plugin (elastic#91067)
  Use correct environment in anomaly detection setup link (elastic#91877)
  [FTSR] Convert to tasks and add jest/api integration suites (elastic#91770)
  [CI] Build and publish storybooks (elastic#87701)
  docs: add PHP agent info to docs (elastic#91773)
  [DOCS] Adds and updates Visualization advanced settings (elastic#91904)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:CI Continuous integration release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.11.2 v7.12.0 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build Storybooks on CI
6 participants