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

Include phase strategy in plan status output #1173

Merged
merged 1 commit into from
Dec 17, 2019

Conversation

yankcrime
Copy link
Member

@yankcrime yankcrime commented Dec 12, 2019

What this PR does / why we need it:
Phase strategy can be defined independently of that for a plan, so this commit includes this in the output so that the Operator behaviour is less confusing to the user.

The output of that command now looks like this:

Plan(s) for "zookeeper-instance" in namespace "default":
.
└── zookeeper-instance (Operator-Version: "zookeeper-0.2.0" Active-Plan: "deploy")
    ├── Plan deploy (serial strategy) [COMPLETE]
    │   ├── Phase zookeeper (parallel strategy) [COMPLETE]
    │   │   └── Step deploy [COMPLETE]
    │   └── Phase validation (serial strategy) [COMPLETE]
    │       ├── Step validation [COMPLETE]
    │       └── Step cleanup [COMPLETE]
    └── Plan validation (serial strategy) [NOT ACTIVE]
        └── Phase connection (serial strategy) [NOT ACTIVE]
            ├── Step connection [NOT ACTIVE]
            └── Step cleanup [NOT ACTIVE]

Fixes #1171

@yankcrime yankcrime force-pushed the plan_status_output branch 2 times, most recently from 1ac1ffc to 2c1d404 Compare December 13, 2019 13:33
@yankcrime yankcrime added the component/cli kubectl kudo a.k.a. kudoctl label Dec 13, 2019
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

I have two small string nits, when we resolve that, this is good to be merged :)

pkg/kudoctl/cmd/plan/plan_status.go Outdated Show resolved Hide resolved
pkg/kudoctl/cmd/plan/plan_status.go Outdated Show resolved Hide resolved
pkg/kudoctl/cmd/plan/plan_status.go Show resolved Hide resolved
@@ -82,7 +82,7 @@ func TestStatus(t *testing.T) {
.
└── test (Operator-Version: "test-1.0" Active-Plan: "deploy")
└── Plan deploy ( strategy) [FATAL_ERROR]
└── Phase deploy [FATAL_ERROR]
└── Phase deploy ( strategy) [FATAL_ERROR]
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not a very nice output :/ but I guess it's fine for now, I'll fix it once we merge it :)

This actually should never happen in real life... ? because the strategy should is mandatory in OV

Copy link
Member

@ANeumann82 ANeumann82 left a comment

Choose a reason for hiding this comment

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

lgtm

Phase strategy can be defined independently of that for a plan, so this
commit includes this in the output so that the Operator behaviour is
less confusing to the user.
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

Thank you! 👏

@alenkacz alenkacz merged commit e2d383f into kudobuilder:master Dec 17, 2019
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
Phase strategy can be defined independently of that for a plan, so this
commit includes this in the output so that the Operator behaviour is
less confusing to the user.

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cli kubectl kudo a.k.a. kudoctl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output of plan status can be confusing where strategy for phase / step differs to the plan
3 participants