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

Rollback feature #221

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Rollback feature #221

wants to merge 3 commits into from

Conversation

theghost5800
Copy link
Contributor

Description:

Issue:

cf rollback-mta -i OPERATION_ID -a ACTION [-u URL]`,
Options: map[string]string{
deployServiceURLOpt: "Deploy service URL, by default 'deploy-service.<system-domain>'",
versionRuleOpt: "Version rule (HIGHER, SAME_HIGHER, ALL)",
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need to provide version rule ALL, in order the rollback to work?
I guess users will rollback to previous versions?

func (c *RollbackMtaCommand) GetPluginCommand() plugin.Command {
return plugin.Command{
Name: "rollback-mta",
HelpText: "Rollback of a multi-target app",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can add more info that it will work only if the user has specified --backup-existing-apps in the previous deploy

Copy link
Contributor

@Yavor16 Yavor16 Dec 13, 2024

Choose a reason for hiding this comment

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

If we do this than should we also add info in the descrption of --backup-existing-apps that if the user wants to rollback they have to use the new command

Comment on lines 234 to 244
// Context("when namespace is provided", func() {
// It("includes the namespace in the request", func() {
// flags.String("namespace", "test-namespace", "")
// fakeServer.AppendHandlers(
// ghttp.CombineHandlers(
// ghttp.VerifyRequest("GET", "/mtas/test-mta-id"),
// ghttp.RespondWith(http.StatusOK, `[]`),
// ),
// ghttp.CombineHandlers(
// ghttp.VerifyRequest("POST", "/mtas/test-mta-id/rollback"),
// ghttp.RespondWith(http.StatusCreated, nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

why are these tests commented out?

applyNamespaceAppRoutesOpt = "apply-namespace-app-routes"
applyNamespaceAsSuffix = "apply-namespace-as-suffix"
maxNamespaceSize = 36
shouldBackupExistingApps = "backup-existing-apps"
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that there is a pattern the names of the variables to like the parameter name and at the end to habe Opt

func (c *RollbackMtaCommand) GetPluginCommand() plugin.Command {
return plugin.Command{
Name: "rollback-mta",
HelpText: "Rollback of a multi-target app",
Copy link
Contributor

@Yavor16 Yavor16 Dec 13, 2024

Choose a reason for hiding this comment

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

If we do this than should we also add info in the descrption of --backup-existing-apps that if the user wants to rollback they have to use the new command

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

Successfully merging this pull request may close these issues.

3 participants