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.

Sign up for GitHub

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

Jump to bottom

Revert "[Response Ops][Alerting] Backfill actions schema changes for intermediate release (#203184)" #204218

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Revert "[Response Ops][Alerting] Backfill actions schema changes for intermediate release (#203184)" #204218

merged 1 commit into from
Dec 13, 2024

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Dec 13, 2024 •
edited by kibanamachine
Loading

This reverts commit b9bac16.

Summary

Reverting to investigate Failed to poll for work: Invalid interval "PT1M". Intervals must be of the form {number}m. Example: 5m. logs which may be caused by the updated task schema

@ymao1 ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v9.0.0 v8.18.0 labels Dec 13, 2024
pmuellr
pmuellr approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM

@pmuellr
Copy link
Member

pmuellr commented Dec 13, 2024

Since we require a review from @elastic/kibana-core , got a question for you.

Background:

We're using schema.duration in one of our SO schemas:

kibana/x-pack/plugins/task_manager/server/saved_objects/schemas/task.ts

Lines 16 to 20 in bd1c00f

schedule: schema.maybe(
schema.object({
interval: schema.duration(),
})
),

schema.duration validates a value like 1m as PT1M:

kibana/packages/kbn-config-schema/src/types/duration_type.test.ts

Lines 67 to 73 in bd1c00f

test('can be a string', () => {
expect(
duration({
defaultValue: '1h',
}).validate(undefined)
).toMatchInlineSnapshot(`"PT1H"`);
});

We upgraded the schema recently: https://github.com/elastic/kibana/pull/203184/files#diff-de6d5e57a9553e7fd1e56beef4ec61e92009ee4d00031fb7ed746d98762590c3

Theory is: on migration, the value from the validate() of that schema is being used as the value in the migrated doc; so 1m gets changed to PT1M. Our code isn't expecting the PT prefix, and so this field has essentially been mangled. Causing lots of errors.

Our previous migration was around 5 months ago, but we didn't have this problem. Could there have been a change to the migrator since then that would cause the behavior we think we might be seeing? https://github.com/elastic/kibana/pull/188758/files#diff-de6d5e57a9553e7fd1e56beef4ec61e92009ee4d00031fb7ed746d98762590c3

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

@ymao1 ymao1 marked this pull request as ready for review December 13, 2024 16:43
@ymao1 ymao1 requested review from a team as code owners December 13, 2024 16:43
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@ymao1 ymao1 added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Dec 13, 2024
TinaHeiligers
TinaHeiligers approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM

@ymao1 ymao1 merged commit 42693ca into elastic:main Dec 13, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12320258399

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 13, 2024
@ymao1
Revert "[Response Ops][Alerting] Backfill actions schema changes for …
a29b44a 
…intermediate release (elastic#203184)" (elastic#204218)

This reverts commit b9bac16.

## Summary

Reverting to investigate `Failed to poll for work: Invalid interval
"PT1M". Intervals must be of the form {number}m. Example: 5m.` logs
which may be caused by the updated task schema

(cherry picked from commit 42693ca)
@kibanamachine kibanamachine mentioned this pull request Dec 13, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 13, 2024
@kibanamachine @ymao1
[8.x] Revert "[Response Ops][Alerting] Backfill actions schema c…
c1be2bc 
…hanges for intermediate release (#203184)" (#204218) (#204266)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Revert "[Response Ops][Alerting] Backfill actions schema changes
for intermediate release (#203184)"
(#204218)](#204218)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ying
Mao","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-13T17:30:12Z","message":"Revert
\"[Response Ops][Alerting] Backfill actions schema changes for
intermediate release (#203184)\" (#204218)\n\nThis reverts commit
b9bac16.\r\n\r\n##
Summary\r\n\r\nReverting to investigate `Failed to poll for work:
Invalid interval\r\n\"PT1M\". Intervals must be of the form {number}m.
Example: 5m.` logs\r\nwhich may be caused by the updated task
schema","sha":"42693caf5f274b9cc3c30d3305961430b31d8ae3","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Alerting","release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.18.0"],"title":"Revert
\"[Response Ops][Alerting] Backfill actions schema changes for
intermediate release
(#203184)\"","number":204218,"url":"https://github.com/elastic/kibana/pull/204218","mergeCommit":{"message":"Revert
\"[Response Ops][Alerting] Backfill actions schema changes for
intermediate release (#203184)\" (#204218)\n\nThis reverts commit
b9bac16.\r\n\r\n##
Summary\r\n\r\nReverting to investigate `Failed to poll for work:
Invalid interval\r\n\"PT1M\". Intervals must be of the form {number}m.
Example: 5m.` logs\r\nwhich may be caused by the updated task
schema","sha":"42693caf5f274b9cc3c30d3305961430b31d8ae3"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204218","number":204218,"mergeCommit":{"message":"Revert
\"[Response Ops][Alerting] Backfill actions schema changes for
intermediate release (#203184)\" (#204218)\n\nThis reverts commit
b9bac16.\r\n\r\n##
Summary\r\n\r\nReverting to investigate `Failed to poll for work:
Invalid interval\r\n\"PT1M\". Intervals must be of the form {number}m.
Example: 5m.` logs\r\nwhich may be caused by the updated task
schema","sha":"42693caf5f274b9cc3c30d3305961430b31d8ae3"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Ying Mao <[email protected]>
ymao1 added a commit to ymao1/kibana that referenced this pull request Dec 17, 2024
@ymao1
Reapply "[Response Ops][Alerting] Backfill actions schema changes for…
5dd56fd 
… intermediate release (elastic#203184)" (elastic#204218)

This reverts commit 42693ca.
@ymao1 ymao1 mentioned this pull request Dec 17, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TinaHeiligers TinaHeiligers TinaHeiligers approved these changes

@pmuellr pmuellr pmuellr approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ymao1 @pmuellr @elasticmachine @kibanamachine @TinaHeiligers