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

docs: add KeptnTask ref page; enhance guide chapter for keptn-no-k8s #2103

Merged
merged 34 commits into from
Oct 18, 2023

Conversation

StackScribe
Copy link
Contributor

@StackScribe StackScribe commented Sep 15, 2023

This PR and #2089 replace #2086 .

This PR:

  • Expands the content of the "Keptn for non-Kubernetes Applications" guide chapter a bit.
  • Creates a reference page for KeptnTask in the yaml-crd-ref section. Normally, KeptnTask is generated automatically but, in this case, the user must populate and maintain it manually so we need to document the YAML syntax.
  • The reference page no longer has an example but instead links to the guide chapter. We do not need to be maintaining two examples and, in this case, it makes more sense to have the KeptnTask example presented in the context of running a KeptnTask for a non-k8s deployment.

28 September review notes:

  • The initial draft was based on outdated API info. That has been corrected.
  • Please verify that I got the descriptions of the fields correct on the reference page. I improved a bit.
  • I think some steps are missing from the guide procedure:
    • When I apply my KeptnTask resource from my KinD cluster, how does it know which VM/whatever to target?
    • Instructions say nothing about populating KeptnApp or workload resources yet the KeptnTask yaml file does have fields for these. How does the user know what value to use for those fields? Or maybe some of these fields are ignored in this case? We need to clarify this.
    • What about KeptnTask fields (such as timeout) that are also in KeptnTaskDefintion? If the user uses these, do they need to be sure that KeptnTask and the corresponding KeptnTaskDefinition assign the same value to that field or does Keptn reconcile them or does one of them take precedence over the other?

@StackScribe StackScribe added the documentation Improvements or additions to documentation label Sep 15, 2023
@StackScribe StackScribe added this to the 0.9 milestone Sep 15, 2023
@StackScribe StackScribe self-assigned this Sep 15, 2023
@StackScribe StackScribe requested review from a team as code owners September 15, 2023 04:38
@netlify
Copy link

netlify bot commented Sep 15, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 9b4c76a
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/652e5bc9fbd79d000805be7a
😎 Deploy Preview https://deploy-preview-2103--keptn-lifecycle-toolkit.netlify.app/docs/yaml-crd-ref/task
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
@StackScribe StackScribe changed the title docs: KeptnTask ref page; enhance guide chap for keptn-no-k8s docs: add KeptnTask ref page; enhance guide chap for keptn-no-k8s Sep 15, 2023
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
docs/content/en/docs/implementing/tasks-non-k8s-apps.md Outdated Show resolved Hide resolved
docs/content/en/docs/implementing/tasks-non-k8s-apps.md Outdated Show resolved Hide resolved
docs/content/en/docs/implementing/tasks-non-k8s-apps.md Outdated Show resolved Hide resolved
docs/content/en/docs/yaml-crd-ref/task.md Outdated Show resolved Hide resolved
docs/content/en/docs/yaml-crd-ref/task.md Outdated Show resolved Hide resolved
docs/content/en/docs/yaml-crd-ref/task.md Outdated Show resolved Hide resolved
docs/content/en/docs/yaml-crd-ref/task.md Outdated Show resolved Hide resolved
docs/content/en/docs/yaml-crd-ref/task.md Outdated Show resolved Hide resolved
docs/content/en/docs/yaml-crd-ref/task.md Outdated Show resolved Hide resolved
Signed-off-by: Meg McRoberts <[email protected]>
@StackScribe StackScribe marked this pull request as draft September 26, 2023 07:35
Signed-off-by: Meg McRoberts <[email protected]>
@mowies mowies changed the title docs: add KeptnTask ref page; enhance guide chap for keptn-no-k8s docs: add KeptnTask ref page; enhance guide chapter for keptn-no-k8s Sep 26, 2023
@StackScribe StackScribe marked this pull request as ready for review September 28, 2023 08:04
Signed-off-by: Meg McRoberts <[email protected]>
StackScribe and others added 5 commits September 29, 2023 00:44
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: RealAnna <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: RealAnna <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
StackScribe and others added 6 commits October 15, 2023 17:41
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
agardnerIT
agardnerIT previously approved these changes Oct 16, 2023
bacherfl
bacherfl previously approved these changes Oct 16, 2023
StackScribe and others added 3 commits October 17, 2023 03:01
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
Signed-off-by: Meg McRoberts [email protected]

Co-authored-by: odubajDT <[email protected]>
Signed-off-by: Meg McRoberts <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@RealAnna RealAnna merged commit 066be3e into keptn:main Oct 18, 2023
17 checks passed
Keptn Tasks can be triggered for workloads and applications
that are deployed outside of Kubernetes.
For example, Keptn could trigger load and performance tests
for an application that is deployed on a virtual machine.
Copy link
Member

Choose a reason for hiding this comment

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

I would add a sentence here to underline the fact that keptn still needs to be installed on a k8s cluster. I think that would make the general scenario a little easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it "Keptn Tasks running on a Kubernetes cluster..." and some other adjustments to make this more explicit.

For example:
When you have Keptn installed, create a
YAML file that defines what you want to execute
as a `KeptnTaskDefinition` resource..
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
as a `KeptnTaskDefinition` resource..
as a `KeptnTaskDefinition` resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


Here though, we must create it ourselves.
Moreover, each time you want to execute a `KeptnTask`,
you must manually create a a new (and uniquely named) `KeptnTask` resource.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
you must manually create a a new (and uniquely named) `KeptnTask` resource.
you must manually create a new (and uniquely named) `KeptnTask` resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +7 to +12
When using Keptn to run tasks for software
that is deployed outside of Kubernetes,
you must create the `KeptnTask` resource manually
and modify it manually for each new run.
Keptn automatically populates the `KeptnTask` resource
for tasks that deploy software on Kubernetes.
Copy link
Member

Choose a reason for hiding this comment

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

I would switch this paragraph around so that it first tells users the "happy path" which is that they don't need to anything manually with KeptnTasks. And then afterwards, I would add the info about handling apps outside of k8s.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

* **apiVersion** -- API version being used.
`
* **kind** -- Resource type.
Must be set to `KeptnTask`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Must be set to `KeptnTask`
Must be set to `KeptnTask`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

## Fields

* **apiVersion** -- API version being used.
`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@StackScribe
Copy link
Contributor Author

Some comments came in after this PR was merged. These are applied in #2293 .

@StackScribe StackScribe deleted the 0914-non-k8s-apps branch October 18, 2023 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants