Add Inline YAML Program Functionality #336

roothorp · 2022-10-07T15:20:07Z

Fixes #312

squaremo · 2022-10-10T09:51:32Z

Test cases:

Can create the Program value and savee it; and create a Stack that refers to it
A valid Program/Stack pair will run the stack (make the Program do something minimal, though)
- need to save kubeconfig and refer to it in env as KUBECONFIG (examples in other tests)
A Stack refering to a missing Program will fail and be marked ~~"retrying"~~ "stalled" (I think there's a SourceUnavailable reason)
A Program that is syntactically OK but invalid (e.g., dangling variable reference) will cause the Stack to fail and be marked stalled, reason: "incorrect program"

deploy/crds/pulumi.com_programs.yaml

pkg/controller/stack/stack_controller.go

test/program_test.go

squaremo

Please take my comments as suggestions, in general. This is "request changes" because I would like to see comments on the new API fields, so that there are descriptions in the documentation.

deploy/crds/pulumi.com_stacks.yaml

pkg/apis/pulumi/v1/program_types.go

pkg/apis/pulumi/shared/stack_types.go

pkg/controller/stack/stack_controller.go

test/program_test.go

This commit implements the handling of Stacks that refer to Programs, by adding a case for .spec.programRef, a func to setup the workspace, and a special case for rescheduling.

NB: the codegen (via `crdoc`) fails to escape angle braces in the enum values, so docs/programs.md is not quite correct. Ideally we can fix the upstream bug at some point. Signed-off-by: Michael Bridgen <mbridgen@pulumi.com> Co-authored-by: Roo Thorp <roo@pulumi.com>

squaremo

Looks good; there's a couple of cosmetic things that could be changed, but neither is a one way door. Nice work! 🍎

squaremo · 2022-10-14T13:45:02Z

pkg/controller/stack/stack_controller.go

+				reqLogger.Info("New commit hash found", "Current commit", currentCommit,
+					"Last commit", instance.Status.LastUpdate.LastSuccessfulCommit)
+			}
+		}


Note to self: we've kept this repeated section deliberately, for now, to keep the pattern of if-then-else with the various sources. We should revisit the control flow of the whole method, at some point.

squaremo · 2022-10-14T13:45:19Z

pkg/controller/stack/stack_controller.go

@@ -868,6 +905,70 @@ func (sess *reconcileStackSession) SetupWorkdirFromGitSource(ctx context.Context
 	return revision, sess.setupWorkspace(ctx, w)
 }

+// ProjectFile adds required Pulumi 'project' fields to the Program spec, making it valid to be given to Pulumi.


squaremo · 2022-10-14T13:53:07Z

pkg/apis/pulumi/v1/program_types.go

+}
+
+// +kubebuilder:validation:Enum={"String", "Number", "List<Number>", "List<String>"}
+type ConfigTypes string


Suggested change

type ConfigTypes string

type ConfigType string

It would be conventional for the type name to be singular here: ConfigType. The name doesn't form part of the public API (except in so far as it's exported from this package), though.

squaremo · 2022-10-14T13:53:23Z

pkg/apis/pulumi/v1/program_types.go

+type Configuration struct {
+	// type is the (required) data type for the parameter.
+	// +optional
+	Type ConfigTypes `json:"type,omitempty"`


Suggested change

Type ConfigTypes `json:"type,omitempty"`

Type ConfigType `json:"type,omitempty"`

squaremo · 2022-10-14T13:59:06Z

test/program_test.go

+			if strings.HasPrefix(tmpDir, os.TempDir()) {
+				os.RemoveAll(tmpDir)
+			}
+			err := k8sClient.Delete(context.TODO(), &stack)


Suggested change

err := k8sClient.Delete(context.TODO(), &stack)

Expect(k8sClient.Delete(context.TODO(), &stack)).To(Succeed())

squaremo · 2022-10-14T13:59:34Z

test/program_test.go

+			Expect(stack.Status.LastUpdate.State).To(Equal(shared.FailedStackStateMessage))
+			stalledCondition := apimeta.FindStatusCondition(stack.Status.Conditions, pulumiv1.StalledCondition)
+			Expect(stalledCondition).ToNot(BeNil(), "stalled condition is present")
+			Expect(stalledCondition.Reason).To(Equal(pulumiv1.StalledSourceUnavailableReason))


squaremo · 2022-10-14T14:00:57Z

test/program_test.go

+			waitForStackFailure(&stack)
+
+			Expect(stack.Status.LastUpdate.State).To(Equal(shared.FailedStackStateMessage))
+			// TODO: Condition for invalid program?


Generally all we'll know is that the automation API failed to run it to completion; a failed state might be as good as we can do for now.

squaremo · 2022-10-14T16:23:39Z

NB on failing tests: the failures already fail on default branch. We'll try to roll default branch forward to fix them.

roothorp requested a review from squaremo October 7, 2022 15:20

roothorp self-assigned this Oct 7, 2022

lblackstone reviewed Oct 10, 2022

View reviewed changes

deploy/crds/pulumi.com_programs.yaml Show resolved Hide resolved

pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved

test/program_test.go Outdated Show resolved Hide resolved

test/program_test.go Outdated Show resolved Hide resolved

squaremo suggested changes Oct 11, 2022

View reviewed changes

Roo Thorp added 4 commits October 13, 2022 13:37

Add Program API types

28a0cf1

Update Makefile and add docs

3cb646b

Add program tests & functionality to controller

084cf56

This commit implements the handling of Stacks that refer to Programs, by adding a case for .spec.programRef, a func to setup the workspace, and a special case for rescheduling.

Add changelog entry for inline YAML programs

b225013

squaremo force-pushed the roothorp/inline-yaml branch from 7999e10 to 9127fd0 Compare October 13, 2022 13:46

Roo Thorp and others added 3 commits October 13, 2022 14:48

Add extra testcases

86cc8d8

Add changes as per reviews

c97f5d3

squaremo force-pushed the roothorp/inline-yaml branch from 9127fd0 to c0f70d1 Compare October 13, 2022 13:48

Roo Thorp added 3 commits October 14, 2022 13:33

Update Program API to include descriptions

a4d7492

Clean up Program tests with helper functions

873fbb2

Add SourceUnknown reason when Program is nonexistent

5df3bcd

roothorp marked this pull request as ready for review October 14, 2022 13:03

squaremo approved these changes Oct 14, 2022

View reviewed changes

Cleanup as per review

c3c31be

squaremo mentioned this pull request Oct 14, 2022

Make mutual exclusion of sources obvious #340

Merged

roothorp merged commit 72e4168 into master Oct 14, 2022

roothorp deleted the roothorp/inline-yaml branch October 14, 2022 16:11

squaremo mentioned this pull request Oct 18, 2022

Trigger Stack runs when a source is updated #343

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Inline YAML Program Functionality #336

Add Inline YAML Program Functionality #336

roothorp commented Oct 7, 2022

squaremo commented Oct 10, 2022 •

edited

Loading

squaremo left a comment

squaremo left a comment

squaremo Oct 14, 2022

squaremo Oct 14, 2022

squaremo Oct 14, 2022

squaremo Oct 14, 2022

squaremo Oct 14, 2022

squaremo Oct 14, 2022

squaremo Oct 14, 2022

squaremo commented Oct 14, 2022

	Type ConfigTypes `json:"type,omitempty"`
	Type ConfigType `json:"type,omitempty"`

	err := k8sClient.Delete(context.TODO(), &stack)
	Expect(k8sClient.Delete(context.TODO(), &stack)).To(Succeed())

Add Inline YAML Program Functionality #336

Add Inline YAML Program Functionality #336

Conversation

roothorp commented Oct 7, 2022

squaremo commented Oct 10, 2022 • edited Loading

squaremo left a comment

Choose a reason for hiding this comment

squaremo left a comment

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo Oct 14, 2022

Choose a reason for hiding this comment

squaremo commented Oct 14, 2022

squaremo commented Oct 10, 2022 •

edited

Loading