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(bump automation): bump ubi9 for ironbank #191660

Merged
merged 7 commits into from
Oct 10, 2024

Conversation

v1v
Copy link
Member

@v1v v1v commented Aug 28, 2024

Summary

Enable updatecli policies to bump the Ironbank versions automatically, then #182738 won't be manually created but when a new ubit9 version is released and available in the ironbank system.

Those policies can be found at https://github.com/elastic/oblt-updatecli-policies/tree/main/updatecli/policies/ (NOTE: This is a private repository only accessible by Elastic employees)

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

How to test this PR locally

  1. gh pr checkout 191660
  2. Install updatecli
  3. Login to ghcr.io
  4. Diff (dry-run)
$ GITHUB_TOKEN=$(gh auth token) updatecli compose diff --experimental
  1. Create Pull Request if new changes
$ GITHUB_REPOSITORY=elastic/kibana \
   GITHUB_ACTOR=v1v \
   GITHUB_TOKEN=$(gh auth token) \
updatecli compose apply --experimental

@v1v v1v added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting skip-ci labels Aug 28, 2024
@v1v v1v self-assigned this Aug 28, 2024
@v1v v1v requested a review from a team as a code owner August 28, 2024 16:30
@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

.github/updatecli/values.d/scm.yml Outdated Show resolved Hide resolved
updatecli-compose.yaml Outdated Show resolved Hide resolved
@v1v v1v requested a review from reakaleek September 4, 2024 14:15
- uses: docker/login-action@0d4c9c5ea7693da7b068278f7b52bda2a190a446 # v3.2.0
with:
registry: ghcr.io
username: ${{ github.actor }}
Copy link
Contributor

@Ikuni17 Ikuni17 Sep 18, 2024

Choose a reason for hiding this comment

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

Should this be a bot instead? It seems that actor is not well defined, and would run in the context of an employee with their privlages, which doesn't seem to be ideal.

Copy link
Member Author

@v1v v1v Sep 19, 2024

Choose a reason for hiding this comment

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

Interesting. The official docs explained how to use this docker login with the GITHUB_TOKEN:

Maybe the issue with github.actor is not the case for this specific use case. We have used this approach so far without any issues, for instance, see the below build for the same kind of code in a different GitHub repository:

I can see my username, but the following steps do what's expected (download the containers and so on ):

image

Likely, GitHub does something special with the login, and it's not honoured but the GITHUB_TOKEN.

So far I think we are safe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the links. I think you're correct that the actor is not honored and the GITHUB_TOKEN takes precedence. It's just a bit strange.

@v1v v1v enabled auto-merge (squash) September 20, 2024 11:32
@v1v
Copy link
Member Author

v1v commented Sep 20, 2024

/ci

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 20, 2024

💔 Build Failed

Failed CI Steps

History

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

cc @v1v

@v1v
Copy link
Member Author

v1v commented Sep 20, 2024


❌ /.buildkite/scripts/steps/checks/file_casing.sh: 1m 16s | 0s
-- | --
  | warn [quick-checks] --- Check File Casing
  | ERROR Filenames MUST use snake_case.
  | - updatecli-compose.yaml

I'm afraid the name of the file is that

@Ikuni17
Copy link
Contributor

Ikuni17 commented Sep 20, 2024


❌ /.buildkite/scripts/steps/checks/file_casing.sh: 1m 16s | 0s
-- | --
  | warn [quick-checks] --- Check File Casing
  | ERROR Filenames MUST use snake_case.
  | - updatecli-compose.yaml

I'm afraid the name of the file is that

You can add an exception to src/dev/precommit_hook/casing_check_config.js for this file

@v1v
Copy link
Member Author

v1v commented Oct 10, 2024

You can add an exception to src/dev/precommit_hook/casing_check_config.js for this file

Thanks, see 758ce64 (#191660)

@v1v
Copy link
Member Author

v1v commented Oct 10, 2024

@Ikuni17, do you know how can I merge this PR? I don't think it requires to run the kibana-ci, changes are totally unrelated to what the CI does validate. Thanks 🙏

@Ikuni17 Ikuni17 removed the skip-ci label Oct 10, 2024
@Ikuni17
Copy link
Contributor

Ikuni17 commented Oct 10, 2024

@v1v I think due to changes in src/dev/precommit_hook/casing_check_config.js CI wants to run. It will auto merge assuming CI passes.

@v1v v1v merged commit eefabb0 into elastic:main Oct 10, 2024
23 checks passed
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #6 / Header rendering it renders the data providers when show is true
  • [job] [logs] Jest Tests #10 / SyncAlertsToggle it renders

Metrics [docs]

✅ unchanged

cc @v1v

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants