Skip to content
This repository has been archived by the owner on Jun 4, 2021. It is now read-only.

Refactor k8s source to use the sdk. #123

Closed
wants to merge 5 commits into from
Closed

Refactor k8s source to use the sdk. #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f0efac1
Refactor k8s source to use the sdk.
Nov 20, 2018
fecd698
Adding reconciliation tests.
Nov 20, 2018
8578d89
Adding new propagate tests.
Nov 20, 2018
271ebed
use a mark method instead of a propagate.
Nov 20, 2018
9ab7f8a
Fix the controller test.
Nov 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 55 additions & 9 deletions pkg/apis/sources/v1alpha1/kuberneteseventsource_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,14 @@ const (
// KubernetesEventSourceConditionReady has status True when the
// source is ready to send events.
KubernetesEventSourceConditionReady = duckv1alpha1.ConditionReady

// KubernetesEventSourceContainerSourceReady has status True when the
// container source produced as a result of this resource is ready.
KubernetesEventSourceContainerSourceReady = "ContainerSourceReady"
)

var kubernetesEventSourceCondSet = duckv1alpha1.NewLivingConditionSet()
var kubernetesEventSourceCondSet = duckv1alpha1.NewLivingConditionSet(
KubernetesEventSourceContainerSourceReady)

// KubernetesEventSourceStatus defines the observed state of the source.
type KubernetesEventSourceStatus struct {
Expand Down Expand Up @@ -82,16 +87,57 @@ func (s *KubernetesEventSourceStatus) InitializeConditions() {
kubernetesEventSourceCondSet.Manage(s).InitializeConditions()
}

// MarkReady sets the condition that the ContainerSource owned by
// the source has Ready status True.
func (s *KubernetesEventSourceStatus) MarkReady() {
kubernetesEventSourceCondSet.Manage(s).MarkTrue(KubernetesEventSourceConditionReady)
// PropagateContainerSourceStatus examines the given container source and synchronizes the conditions that matter to
// the kubernetes event source status.
func (ss *KubernetesEventSourceStatus) MarkContainerSourceReadyStatus(cs ContainerSourceStatus) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment starts with PropagateContainerSourceStatus, which seems like what this method does, but the method is named MarkContainerSourceReadyStatus.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we expect people to use ContainerSource this way (having their own source wrap it), this method should be moved somewhere it can be shared.

func PropagateContainerSourceStatus(cm ConditionManager, cs ContainerSourceStatus)

Then in this file:

func PropagateContainerSourceStatus(cs ContainerSourceStatus) {
    containersource.PropagateConatinerSourceStatus(kubernetesEventSourceCondSet.Manage(s), cs)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

ss -> s to match the other functions in the file.

if cs.IsReady() {
kubernetesEventSourceCondSet.Manage(ss).MarkTrue(KubernetesEventSourceContainerSourceReady)
return
}

sinkProvided := cs.GetCondition(ContainerConditionSinkProvided)
deployed := cs.GetCondition(ContainerConditionDeployed)

reason := ""
message := ""
isFalse := false
Copy link
Contributor

Choose a reason for hiding this comment

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

How about statusFalse? For some reason isFalse feels wrong to me.


if sinkProvided == nil {
reason = reason + "SinkProvided"
message = appendMessage(message, "SinkProvided status is nil")
} else if sinkProvided.IsUnknown() {
reason = reason + sinkProvided.Reason
message = appendMessage(message, sinkProvided.Message)
} else if sinkProvided.IsFalse() {
reason = reason + sinkProvided.Reason
message = appendMessage(message, sinkProvided.Message)
isFalse = true
}

if deployed == nil {
reason = reason + "Deploy"
message = appendMessage(message, "Deploy status is nil")
} else if deployed.IsUnknown() {
reason = reason + deployed.Reason
message = appendMessage(message, deployed.Message)
} else if deployed.IsFalse() {
reason = reason + deployed.Reason
message = appendMessage(message, deployed.Message)
isFalse = true
}

if isFalse {
kubernetesEventSourceCondSet.Manage(ss).MarkFalse(KubernetesEventSourceContainerSourceReady, reason, message)
} else {
kubernetesEventSourceCondSet.Manage(ss).MarkUnknown(KubernetesEventSourceContainerSourceReady, reason, message)
}
}

// MarkUnready sets the condition that the ContainerSource owned by
// the source does not have Ready status True.
func (s *KubernetesEventSourceStatus) MarkUnready(reason, messageFormat string, messageA ...interface{}) {
kubernetesEventSourceCondSet.Manage(s).MarkFalse(KubernetesEventSourceConditionReady, reason, messageFormat, messageA...)
func appendMessage(a string, b string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using strings.Join instead?

Above would change to something like:

messages := make([]string)
...
messages = append(messages, "SinkProvided status is nil")
...
 MarkUnknown(..., reason, strings.Join(messages, "; "))

if a != "" {
a += "; "
}
return a + b
}

// +genclient
Expand Down
Loading