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

Deprecation plan for serial preview/push #3142

Open
tlimoncelli opened this issue Oct 4, 2024 · 6 comments
Open

Deprecation plan for serial preview/push #3142

tlimoncelli opened this issue Oct 4, 2024 · 6 comments

Comments

@tlimoncelli
Copy link
Contributor

tlimoncelli commented Oct 4, 2024

  1. Release v4.14 (<1 week)
  • Add oldpreview/oldpush commands that are aliases for the serial preview/push with warning that these are temporary, will go away in a future release without warning, and are not subject to SemVer.
    • Draft: "WARNING: oldpreview/oldpush are temporary commands and may go away in a future release without warning. They are not subject to SemVer. Please use ppreview/ppush instead."
  • Add message to preview/push commands:
    • Draft: 'WARNING: In the next release (v4.15), this command will receive a major upgrade. A preview is available by using ppreview/ppush. Please test them and report any bugs ASAP. The older code will be available as oldpreview/oldpush for a limited number of releases.'
  • Add a (similar) message to the v4.14 release notes.
  • Send a mailing to the mailinglist with the release notes (deprecation message included).
  1. Release v4.15 (1 month)
  • preview/push will activate the ppreview/ppush code. Remove any warnings from these commands.
  • oldpreview/oldpush will be available as a temporary work-around if bugs are found in ppreview/ppush. They will output a warning message. Draft:
    • Draft: "WARNING: oldpreview/oldpush are temporary commands and may go away in a future release without warning. They are not subject to SemVer."
  1. Future release (at least 2 months, target date Jan 1, 2025 or later)

    • oldpreview/oldpush will be removed from the code base
@cafferata
Copy link
Collaborator

Crosslinking my original comment #3135 (comment)

@imlonghao
Copy link
Contributor

  1. target date Jan 1, 2024 or later

You mean 2025 right?

@tlimoncelli
Copy link
Contributor Author

  1. target date Jan 1, 2024 or later

You mean 2025 right?

Wooops! You are correct! Thank you for catching that!

@cafferata
Copy link
Collaborator

After my review of #3145 I took another look at the bigger picture. Here is a summary of all the current DNSControl commands:

  1. There are four commands:
    1. preview
    2. push
    3. ppreview
    4. ppush
  2. Within ppreview and ppush there is an additional argument:
    1. cmode: default (Which providers to run concurrently: all, default, none)
      • flags = append(flags, &cli.StringFlag{
        Name: "cmode",
        Destination: &args.ConcurMode,
        Value: "default",
        Usage: `Which providers to run concurrently: all, default, none`,
        Action: func(c *cli.Context, s string) error {
        if !slices.Contains([]string{"all", "default", "none"}, s) {
        fmt.Printf("%q is not a valid option for --cmode. Valie are: all, default, none", s)
        }
        return nil
        },
        })
    2. The argument cmode uses the constant CanConcur which is defined per provider.

What do we want to achieve?

That the codebase is moving forward where DNSControl users will use the new concurrently mode without experiencing too many problems. When introducing this new concurrently feature, a second command was chosen instead of an argument. (I don't know why.) Now we're about to bring ppreview and ppush back as standard.

The impact

We discussed keeping the old functionality available on another/second (alias) so that there remains a fallback method. I think most of the functionality can be guaranteed with cmode. So maybe the following is a better idea:

  1. Release v4.14 (1 week from now)

    • Add a deprecation message within the preview and push commands.
      • Context: 'in de next release v4.15 the commands preview en push will be replaced by ppreview and ppush functionality. We encourage you to test the new ppreview and ppush commands for your own DNSControl set-up/configured providers.'
    • Add a deprecation/warning message within the ppreview and ppush commands.

      The commands will go away when they replace the existing preview/pushcommands.

    • Add a (similar) deprecation messages to the v4.14 release notes.
    • Send a mailing to the mailinglist with the release notes (deprecation message included).
  2. Release v4.15 (+2 weeks from releasing v4.14)

    • The commands preview en push are replaced by ppreview and ppush functionality.
    • The default value of cmode is default (remains unchanged).
    • Add (again) a deprecation/warning message to the v4.15 release notes.
    • Send a mailing to the mailinglist with the release notes (deprecation message included).
    • The fallback for providers without parallel support is still cmode with value none.
  3. Release v4.16 (+3/4 weeks from releasing v4.15)

    • Remove of the (old) ppreview en ppush commands.
    • Add a (breaking change) message to the v4.16 release notes.
      • The ppreview and ppush commands have been removed.
    • Send a mailing to the mailinglist with the release notes (breaking change message included).

v4.14

dnscontrol preview
dnscontrol push
[WARNING: The preview/push commands will receive a major upgrade in the next release (v4.15). Try the new behavior by using ppreview/ppush. Please test and report any bugs ASAP. See https://docs.dnscontrol.org/commands/preview-push]
dnscontrol ppreview
dnscontrol ppush
[WARNING: The ppreview/ppush commands will be removed in the upcoming release (v4.16). See https://docs.dnscontrol.org/commands/preview-push]

v4.15

dnscontrol preview
dnscontrol push

These commands have already been replaced for the ppreview and ppush functionality so a warning is no longer needed.

dnscontrol ppreview
dnscontrol ppush
[WARNING: The ppreview/ppush commands will be removed in the upcoming release (v4.16). See https://docs.dnscontrol.org/commands/preview-push]

v4.16

dnscontrol preview
dnscontrol push

These commands have already been replaced for the ppreview and ppush functionality so a warning is no longer needed.

dnscontrol ppreview
dnscontrol ppush
No help topic for 'ppreview'

This is the standard (error) message for a DNSControl command that does (no longer) not exist.

So I think we can move forward much more easily than I initially thought. What do you think about this?

Crosslinking of the GitHub pull requests/issues:

  1. FEATURE: parallel preview/push is now default #3132
  2. Revert "FEATURE: parallel preview/push is now default (#3132)" #3144
  3. Fix ppush/oldpush docs #3133
  4. DOCS: Explain ppreview/oldpreview changes #3135 (comment)
  5. Deprecation plan for serial preview/push #3142

@tlimoncelli
Copy link
Contributor Author

When introducing this new concurrently feature, a second command was chosen instead of an argument. (I don't know why.)

@cafferata , you are brilliant! This is the root of the problem! That is... I introduced a new command rather than a flag. Flags are a better way to do this than subcommands. (Why didn't I use a flag? Well, at the time I didn't think it would be possible.)

I'd like to note, however, that --cmode none is not the same as running the old serial code. However, I could add a value that would activate the old code.

Then we add to your plan:

Release v4.14: Add --cmode flag to preview/push. Default "legacy".
Permitted values:

  • legacy (old code) (warning: Please try normal, file bugs, etc)
  • normal (abide by Can/Cannot)
  • none (forced no concurrence)
  • all (forced concurrence),

Release v4.15: --cmode defaults to "normal"

  • legacy generates a warning "This will go away in v4.16 or later, target 1-Jan-2025)

Release v4.16 (or later): Remove the "legacy" option (and the legacy code)

The benefit of this is that the old code is available for longer, but it doesn't require people to change scripts / CICD configs, etc.

What do you think?

@cafferata
Copy link
Collaborator

I'd like to note, however, that --cmode none is not the same as running the old serial code. However, I could add a value that would activate the old code.

What do you think?

If you see technical possibilities for that I would say, Yes! Let's try/get this ready because that would make the step forward (even) easier. 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants