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

feat(config): Add framework for migrating deprecated plugins #13377

Merged
merged 11 commits into from
Jun 9, 2023

Conversation

srebhan
Copy link
Member

@srebhan srebhan commented Jun 1, 2023

relates to ##13375
relates to ##13376

This PR adds a framework for migrating configurations containing deprecated plugins (and options). For new the use is demonstrated for the inputs.cassandra input plugin. The PR adds a migrate sub-command to the config command.

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 1, 2023
@srebhan
Copy link
Member Author

srebhan commented Jun 1, 2023

@powersj assigning this to you to get your feedback and ideas...

Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

I like the general layout of this all. Thank you

One comment about documenting the boolean. Can we do test cases for the cassandra migration as well?

config/config.go Show resolved Hide resolved
@srebhan srebhan requested a review from powersj June 6, 2023 08:18
@srebhan srebhan changed the title feat: Add framework for migrating deprecated plugins feat(config): Add framework for migrating deprecated plugins Jun 6, 2023
@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jun 6, 2023
@powersj
Copy link
Contributor

powersj commented Jun 6, 2023

@srebhan couple errors from tests: one lint error + new test cases error

@srebhan srebhan removed the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jun 6, 2023
@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jun 6, 2023
@telegraf-tiger
Copy link
Contributor

telegraf-tiger bot commented Jun 6, 2023

return buf, nil
}

func (j *jolokiaAgent) fillCommon(o common.InputOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

will we need to a) keep this up to date with all "common" options and b) have this in each migration? or will you generalize it out after the next one?

Copy link
Member Author

@srebhan srebhan Jun 9, 2023

Choose a reason for hiding this comment

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

I would have loved to generalize this but I ran into the issue of TOML marshalling cannot marshal an embedded struct to the same level as the embedding struct. I.e. if you have

type A struct {
    fielda string `toml:"a"
}

type B struct {
    fieldb1 string `toml:"b1"`
    fieldb2 string `toml:"b2"`
    A
}

you will get

[[inputs.B]]
    b1 = "foo"
    b2 = "bar"
    [inputs.B.A]
      a = "fail"

instead of

[[inputs.B]]
   b1 = "foo"
   b2 = "bar"
   a = "good"

Therefore I cannot generalize it and we need to do this in every migration or fix it in the TOML marshaller...

Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

One question, but should not hold up merging this.

@powersj powersj assigned srebhan and unassigned powersj Jun 7, 2023
@srebhan srebhan merged commit 16786d2 into influxdata:master Jun 9, 2023
@srebhan srebhan deleted the migrations branch June 9, 2023 08:10
@srebhan srebhan added this to the v1.27.0 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants