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

procedures: Add readiness init container docs #2139

Merged
merged 37 commits into from
Nov 26, 2021

Conversation

mmorhun
Copy link
Contributor

@mmorhun mmorhun commented Oct 18, 2021

Signed-off-by: Mykola Morhun mmorhun@redhat.com

What does this pull request change?

What issues does this pull request fix or reference?

eclipse-che/che#20337

Specify the version of the product this pull request applies to

7.38 and above

Pull Request checklist

The author and the reviewers validate the content of this pull request with the following checklist, in addition to the automated tests.

  • Any procedure:
    • Successfully tested.
  • Any page or link rename:
  • Builds on Eclipse Che hosted by Red Hat.
  • the Validate language on files added or modified step reports no vale warnings.

@mmorhun mmorhun requested review from rkratky, themr0c and a team as code owners October 18, 2021 13:44
@mmorhun mmorhun self-assigned this Oct 18, 2021
@github-actions
Copy link

Click here to review and test in web IDE: Contribute

Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@tolusha
Copy link
Contributor

tolusha commented Oct 19, 2021

I expect having jq/yq commands to update resources.

@MichalMaler MichalMaler self-assigned this Oct 21, 2021
@MichalMaler MichalMaler added this to the 7.37 milestone Oct 21, 2021
@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@MichalMaler MichalMaler marked this pull request as draft October 21, 2021 13:08
@mmorhun
Copy link
Contributor Author

mmorhun commented Oct 26, 2021

@tolusha it is not trivial task. There are a few problem here:

  • the env var could be present or not. So we have to handle both cases and there is no ifs or add/change policy.
  • It is not trivial to work in patch with an array if we need to select one by an inner property, not its key
  • the target env var inside CSV cannot be changed with a patch

So, I'd prefer to leave it as is.

@tolusha
Copy link
Contributor

tolusha commented Oct 26, 2021

Ok

@MichalMaler MichalMaler marked this pull request as ready for review October 26, 2021 11:55
@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@themr0c themr0c modified the milestones: 7.37, 7.38 Oct 28, 2021
Signed-off-by: Michal Maléř <mmaler@redhat.com>
Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@github-actions
Copy link

Click here to review and test in web IDE: Contribute

@themr0c themr0c changed the title Add readiness init container docs docs: Add readiness init container docs Nov 24, 2021
@themr0c themr0c changed the title docs: Add readiness init container docs procedures: Add readiness init container docs Nov 24, 2021
Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
…sabling-readiness-init-containers-for-the-olm-installer.adoc

Co-authored-by: Max Leonov <mleonov@redhat.com>
Co-authored-by: Max Leonov <mleonov@redhat.com>
Co-authored-by: Max Leonov <mleonov@redhat.com>
Copy link
Contributor

@max-cx max-cx left a comment

Choose a reason for hiding this comment

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

Language and modular docs reviewed.

@themr0c
Copy link
Contributor

themr0c commented Nov 26, 2021

eclipsefdn/eca is failing because of a commit by Anonymous (39771996+che-bot@****.noreply). I believe it's fine.

@mmorhun mmorhun merged commit 62ffbe2 into eclipse-che:master Nov 26, 2021
@mmorhun mmorhun deleted the che-20337 branch November 26, 2021 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants