Skip to content

Add documentation for pod-overrides and container-overrides attributes #70

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

Jump to bottom
Merged
mike-hoang merged 8 commits into from
Mar 2, 2023
Merged

Add documentation for pod-overrides and container-overrides attributes #70

mike-hoang merged 8 commits into from
Mar 2, 2023

Conversation

@valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented Jan 18, 2023

Signed-off-by: Parthvi Vala pvala@redhat.com

What does this PR do / why we need it

This PR adds documentation for container-overrides and pod-overrides attributes.

Which issue(s) does this PR fix

Fixes devfile/api#1005

PR acceptance criteria

  • Unit Tests
  • E2E Tests
  • Documentation

How to test changes / Special notes to the reviewer

@nx-cloud
Copy link

nx-cloud bot commented Jan 18, 2023 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 45e9d74. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 5 targets

Sent with 💌 from NxCloud.

@openshift-ci openshift-ci bot requested review from elsony and schultzp2020 January 18, 2023 07:25
mike-hoang
mike-hoang reviewed Jan 24, 2023
View reviewed changes
Copy link
Contributor

@mike-hoang mike-hoang left a comment

Choose a reason for hiding this comment

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

The sidebar also needs to be updated if a new file is added: https://github.com/devfile/devfile-web/tree/main/apps/landing-page#configuring-navigation but otherwise everything else looks good to me

@valaparthvi valaparthvi mentioned this pull request Jan 31, 2023
Merged
@valaparthvi valaparthvi force-pushed the 1005_document_pod_container_overrides branch 4 times, most recently from 69bdf32 to 9a6886c Compare February 2, 2023 04:29
kadel
kadel reviewed Feb 6, 2023
View reviewed changes
libs/docs/src/docs/2.2.1-alpha/overriding-pod-and-container-attributes.md

## container-overrides

`container-overrides` attributes allow you to override properties of a container in a pod spec such as `securityContext` and `resources`. However, it restricts from overriding properties such as `image`, `name`, `ports`, `env`, `volumesMounts`, `command`, and `args`.
Copy link
Member

@kadel kadel Feb 6, 2023

Choose a reason for hiding this comment

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

Have other implementations, like in DevWorkspace Operator, the same restrictions?

If not, than it needs to be mentioned

Copy link
Contributor Author

@valaparthvi valaparthvi Feb 6, 2023

Choose a reason for hiding this comment

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

At the time of implementation, DWO had the same restriction. I'll check again and update the doc.

@openshift-ci openshift-ci bot added the lgtm label Feb 7, 2023
kadel
kadel suggested changes Feb 7, 2023
View reviewed changes
Copy link
Member

@kadel kadel left a comment

Choose a reason for hiding this comment

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

This should be included in 2.2.0, 2.1.0, 2.0.0 as well as in 2.2.1-alpha.

The reason is that this is an attribute that doesn't affect Devfile schema, and we support it in all supported versions.

libs/docs/src/docs/2.2.1-alpha/overriding-pod-and-container-attributes.md Outdated
The data inside `container-overrides` can also be specified as a JSON.
```yaml
container-overrides:
securityContext: {"runAsUser": "1001", "runAsGroup":"1001"}
Copy link
Member

@kadel kadel Feb 7, 2023

Choose a reason for hiding this comment

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

this is not working.

Error occurred on Push - watch command was unable to push component: unable to create or update component: failed to parse container-overrides attribute on component tools: json: cannot unmarshal string into Go struct field SecurityContext.securityContext.runAsGroup of type int64

The uid and guid needs to be number not string

libs/docs/src/docs/2.2.1-alpha/overriding-pod-and-container-attributes.md
components:
- name: maven
attributes:
pod-overrides:
Copy link
Member

@kadel kadel Feb 7, 2023

Choose a reason for hiding this comment

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

I wanted to test this, but it doesn't seem to work :-(

Copy link
Contributor Author

@valaparthvi valaparthvi Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You're right. I've created a fix devfile/library#162.

@openshift-ci openshift-ci bot removed the lgtm label Feb 7, 2023
@schultzp2020 schultzp2020 removed their request for review February 7, 2023 14:23
@valaparthvi valaparthvi mentioned this pull request Feb 13, 2023
Merged
5 tasks
@kadel kadel removed their assignment Feb 13, 2023
@valaparthvi valaparthvi requested review from mike-hoang and removed request for elsony February 20, 2023 09:26
mike-hoang
mike-hoang reviewed Feb 21, 2023
View reviewed changes
libs/docs/src/docs/2.2.0/overriding-pod-and-container-attributes.md
@@ -0,0 +1,78 @@
---
Copy link
Contributor

@mike-hoang mike-hoang Feb 21, 2023

Choose a reason for hiding this comment

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

just need to add the overriding-pod-and-container-attributes.md file to the corresponding versions of the sidebar navigation and then it's good to go

for example, 2.2.0 would be located here: https://github.com/devfile/devfile-web/blob/main/libs/docs/src/navigation/2.2.0.yaml

valaparthvi and others added 7 commits March 2, 2023 16:11
@valaparthvi
Add documentation for pod-overrides and container-overrides attributes
e407e7b 
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi @mike-hoang
Update libs/docs/src/docs/2.2.1-alpha/overriding-pod-and-container-at…
796ecd5 
…tributes.md

Co-authored-by: Michael Hoang <35011707+mike-hoang@users.noreply.github.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi
Correct container-attributes doc and other changes
ba50ee1 
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi
Add note about JSON data
aed2233 
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi
Fix typo
f2cf7ca 
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi
Fix container-overrides devfile example and add the same doc for 2.1.…
cef3bae 
…0 and 2.2.0

Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi
Add the doc link to navigation.yaml
3136fb4 
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi valaparthvi force-pushed the 1005_document_pod_container_overrides branch from e4f9bcd to 3136fb4 Compare March 2, 2023 10:50
@mike-hoang
fix invalid links
45e9d74 
Signed-off-by: Michael Hoang <mhoang@redhat.com>
@openshift-ci openshift-ci bot added the lgtm label Mar 2, 2023
@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mike-hoang, valaparthvi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Mar 2, 2023
@mike-hoang mike-hoang merged commit 78d2fda into devfile:main Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@kadel kadel kadel requested changes

@mike-hoang mike-hoang mike-hoang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mike-hoang mike-hoang

Labels

approved lgtm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Document pod-overrides and container-overrides

3 participants

@valaparthvi @kadel @mike-hoang