Skip to content

pod completion status-1 #185

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

Closed
wants to merge 11 commits into from
Closed

pod completion status-1 #185

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
216a7c2
pod completion status-1
asm582 Jun 7, 2022
06a983d
test case
asm582 Jun 8, 2022
c82eddc
completion req flag
asm582 Jun 28, 2022
0fceae5
Add two new states
asm582 Jul 4, 2022
3b3eab7
remove noisy logs
asm582 Jul 4, 2022
b7df0ac
delete log
asm582 Jul 4, 2022
464443b
add debug method
asm582 Jul 18, 2022
17c295d
add debug 1
asm582 Jul 18, 2022
5cf1fc1
add debug 2
asm582 Jul 18, 2022
9779863
changes for pr review
asm582 Jul 19, 2022
23888e4
add delay for test
asm582 Jul 19, 2022
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions config/crd/bases/mcad.ibm.com_appwrappers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,28 @@ spec:
recognized schemas to the latest internal value, and may
reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
# completionrequired:
# description: Optional field CompletionRequired defines if an item in an
# Appwrapper should be a candidate to complete an appwrapper.
# type: boolean
completionstatus:
description: Optional section that drives completion status of appwrapper
items:
properties:
statuspath:
type: string
completionrequired:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do we do with this whole completionstatus stanza if this field is false?

Also, could you add a description for this field? This will improve our documentation. Thanks!
Copy link
Member Author

Choose a reason for hiding this comment

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

If CompletionRequired is false that item will not participate in whole appwrapper completion
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the CompletionRequired is false should we require completionconditions in the required fields? How would the MCAD behavior change if this field is false vs not including the completionstatus stanza at all?
type: boolean
completionconditions:
items:
type: string
type: array
required:
- statuspath
- completionrequired
- completionconditions
type: object
type: array
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment after this line to indicate associated field such as: # Type for completionstatus field?

Is there a reason this should be an array of completionstatus'? I would expect that since this is 1 genericitme object we would have only one way to check for completion? Please let me know your thoughts.
Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I will add a comment about the type.
Completion status has 3 properties as part of represented in array format. statuspath is the yaml path to pickup conditions, completion required is a boolean value that tells if an item in an appwrapper is required to be completed and completion conditions is an array inside an array that can tell which status ie pending, failed are used to determine if an item in appwrapper is completed.Please see example below:

 completionstatus: #actual jsonpath : .status.conditions[0].type - statuspath: "status.conditions.type" completionrequired: true completionconditions: ["Complete", "Failed"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes I see. My question is if the completionstatus needs to be an array completionstatus[].

I could see that it would need to be an array if completionconditions was not an array and there had to be multiple items in completionstatus[], completionstatus[0]forcompletionconditions; "Completed" and completionstatus[1] for completionconditions; "Failed". In the current schema a valid definition is the following:

 completionstatus: - statuspath: "status.conditions.type" completionrequired: true completionconditions: ["Complete", "Failed"] - statuspath: "status.someother.state" completionrequired: true completionconditions: ["Finished"]

I assume the 2 items in completionstatus would be ANDed. And the completionconditions is ORed. Since the assumption is that the field for statuspath: is assumed to be an array we should clarify this in the schema description if not done already. Or you could add a statuspathtype which could indicate an array or string or number.
Copy link
Collaborator

@dmatch01 dmatch01 Jul 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Alternate example:
completionstatus:
- statuspath: "status.conditions.type"
completionconditions: "Complete"
- statuspath: "status.conditions.type"
completionconditions: "Failed"
- statuspath: "status.someother.state"
completionconditions: "Finished"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Otherwise change completionstatus: to an object...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest open new issue to generalize statuspath: and remove from this PR.
custompodresources:
description: Optional section that specifies resource requirements
for non-standard k8s resources, follows same format as
Expand Down
22 changes: 22 additions & 0 deletions deployment/mcad-controller/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7027,6 +7027,28 @@ spec:
recognized schemas to the latest internal value, and may
reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
# completionrequired:
# description: Optional field CompletionRequired defines if an item in an
# Appwrapper should be a candidate to complete an appwrapper.
# type: boolean
completionstatus:
description: Optional section that drives completion status of appwrapper
items:
properties:
statuspath:
type: string
completionrequired:
type: boolean
completionconditions:
items:
type: string
type: array
required:
- statuspath
- completionrequired
- completionconditions
type: object
type: array
custompodresources:
description: Optional section that specifies resource requirements
for non-standard k8s resources, follows same format as
Expand Down
43 changes: 28 additions & 15 deletions pkg/apis/controller/v1beta1/appwrapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ limitations under the License.
package v1beta1

import (
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
)
Expand Down Expand Up @@ -171,6 +171,9 @@ type AppWrapperGenericResource struct {
//Optional section that specifies resource requirements for non-standard k8s resources, follows same format as that
// of standard k8s resources
CustomPodResources []CustomPodResourceTemplate `json:"custompodresources,omitempty"`

//Optional field for users to determine completion status of item
CompletionStatus []CompletionStatusTemplate `json:"completionstatus,omitempty"`
}

type CustomPodResourceTemplate struct {
Expand All @@ -183,6 +186,12 @@ type CustomPodResourceTemplate struct {
Limits v1.ResourceList `json:"limits"`
}

type CompletionStatusTemplate struct {
StatusPath string `json:"statuspath"`
CompletionRequired bool `json:"completionrequired,omitempty"`
CompletionConditions []string `json:"completionconditions,omitempty"`
}

// App Wrapper resources type
type ResourceType string

Expand Down Expand Up @@ -260,25 +269,29 @@ type AppWrapperState string

//enqueued, active, deleting, succeeded, failed
const (
AppWrapperStateEnqueued AppWrapperState = "Pending"
AppWrapperStateActive AppWrapperState = "Running"
AppWrapperStateDeleted AppWrapperState = "Deleted"
AppWrapperStateFailed AppWrapperState = "Failed"
AppWrapperStateEnqueued AppWrapperState = "Pending"
AppWrapperStateActive AppWrapperState = "Running"
AppWrapperStateDeleted AppWrapperState = "Deleted"
AppWrapperStateFailed AppWrapperState = "Failed"
AppWrapperStateCompleted AppWrapperState = "Completed"
AppWrapperStateRunningHoldCompletion AppWrapperState = "RunningHoldCompletion"
)

type AppWrapperConditionType string

const (
AppWrapperCondInit AppWrapperConditionType = "Init"
AppWrapperCondQueueing AppWrapperConditionType = "Queueing"
AppWrapperCondHeadOfLine AppWrapperConditionType = "HeadOfLine"
AppWrapperCondBackoff AppWrapperConditionType = "Backoff"
AppWrapperCondDispatched AppWrapperConditionType = "Dispatched"
AppWrapperCondRunning AppWrapperConditionType = "Running"
AppWrapperCondPreemptCandidate AppWrapperConditionType = "PreemptCandidate"
AppWrapperCondPreempted AppWrapperConditionType = "Preempted"
AppWrapperCondDeleted AppWrapperConditionType = "Deleted"
AppWrapperCondFailed AppWrapperConditionType = "Failed"
AppWrapperCondInit AppWrapperConditionType = "Init"
AppWrapperCondQueueing AppWrapperConditionType = "Queueing"
AppWrapperCondHeadOfLine AppWrapperConditionType = "HeadOfLine"
AppWrapperCondBackoff AppWrapperConditionType = "Backoff"
AppWrapperCondDispatched AppWrapperConditionType = "Dispatched"
AppWrapperCondRunning AppWrapperConditionType = "Running"
AppWrapperCondPreemptCandidate AppWrapperConditionType = "PreemptCandidate"
AppWrapperCondPreempted AppWrapperConditionType = "Preempted"
AppWrapperCondDeleted AppWrapperConditionType = "Deleted"
AppWrapperCondFailed AppWrapperConditionType = "Failed"
AppWrapperCondCompleted AppWrapperConditionType = "Completed"
AppWrapperCondRunningHoldCompletion AppWrapperConditionType = "RunningHoldCompletion"
)

// DeploymentCondition describes the state of a deployment at a certain point.
Expand Down
Loading