Skip to content

STOR-1445: Sync 10_deployment-hypershift.yaml from cluster-storage-operator repo - #2936

Closed
mpatlasov wants to merge 1 commit into
openshift:mainfrom
mpatlasov:sync-CSO-deployment-hypershift.yaml
Closed

STOR-1445: Sync 10_deployment-hypershift.yaml from cluster-storage-operator repo#2936
mpatlasov wants to merge 1 commit into
openshift:mainfrom
mpatlasov:sync-CSO-deployment-hypershift.yaml

Conversation

@mpatlasov

@mpatlasov mpatlasov commented Aug 22, 2023 •
edited by openshift-ci Bot
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

The PR has a commit:

STOR-1445: Add commnad-line option --terminate-on-files to operators dependent on metrics-serving-cert

which restarts CSO if the secret cluster-storage-operator-serving-cert is updated. This PR simply syncs those changes to hypershift repo asset yaml.

/cc @openshift/storage

Checklist

  • [*] Subject and description added to both, commit and PR.
  • [*] Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@mpatlasov
STOR-1445: Sync 10_deployment-hypershift.yaml from `cluster-storage…
67ace58 
…-operator` repo

The operator mounts secrets (with `tls.key` and `tls.cert`) and need to be restarted if they change.
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 22, 2023
@openshift-ci-robot

openshift-ci-robot commented Aug 22, 2023 •
edited by openshift-ci Bot
Loading

Copy link
Copy Markdown

@mpatlasov: This pull request references STOR-1445 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

The PR has a commit:

STOR-1445: Add commnad-line option --terminate-on-files to operators dependent on metrics-serving-cert

which restarts CSO if the secret cluster-storage-operator-serving-cert is updated. This PR simply syncs those changes to hypershift repo asset yaml.

/cc @openshift/storage

Checklist

  • [*] Subject and description added to both, commit and PR.
  • [*] Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

1 similar comment
@openshift-ci-robot

openshift-ci-robot commented Aug 22, 2023 •
edited by openshift-ci Bot
Loading

Copy link
Copy Markdown

@mpatlasov: This pull request references STOR-1445 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

The PR has a commit:

STOR-1445: Add commnad-line option --terminate-on-files to operators dependent on metrics-serving-cert

which restarts CSO if the secret cluster-storage-operator-serving-cert is updated. This PR simply syncs those changes to hypershift repo asset yaml.

/cc @openshift/storage

Checklist

  • [*] Subject and description added to both, commit and PR.
  • [*] Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci

openshift-ci Bot commented Aug 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@mpatlasov: GitHub didn't allow me to request PR reviews from the following users: openshift/storage.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

What this PR does / why we need it:

The PR has a commit:

STOR-1445: Add commnad-line option --terminate-on-files to operators dependent on metrics-serving-cert

which restarts CSO if the secret cluster-storage-operator-serving-cert is updated. This PR simply syncs those changes to hypershift repo asset yaml.

/cc @openshift/storage

Checklist

  • [*] Subject and description added to both, commit and PR.
  • [*] Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci Bot added do-not-merge/needs-area area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release labels Aug 22, 2023
@openshift-ci

openshift-ci Bot commented Aug 22, 2023

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mpatlasov
Once this PR has been reviewed and has the lgtm label, please assign sjenning for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@mpatlasov

mpatlasov commented Aug 23, 2023

Copy link
Copy Markdown
Contributor Author

/test e2e-kubevirt-aws-ovn

@openshift-ci

openshift-ci Bot commented Aug 23, 2023

Copy link
Copy Markdown
Contributor

@mpatlasov: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@mpatlasov

mpatlasov commented Aug 23, 2023

Copy link
Copy Markdown
Contributor Author

/assign @csrwng
(this PR is similar to #2915)

@dobsonj

dobsonj commented Aug 28, 2023

Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2023
@mpatlasov

mpatlasov commented Sep 12, 2023

Copy link
Copy Markdown
Contributor Author

@csrwng , this PR is similar to #2915, can you please approve? Thanks!

csrwng
csrwng reviewed Sep 19, 2023
View reviewed changes
Comment thread ...l-plane-operator/controllers/hostedcontrolplane/storage/assets/10_deployment-hypershift.yaml
- args:
- start
- -v=2
- --terminate-on-files=/var/run/secrets/serving-cert/tls.crt

@csrwng csrwng Sep 19, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see where this is mounted into the pod

@mpatlasov mpatlasov Sep 19, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for nice catch, @csrwng ! It is mounted in standalone version of deployment (https://github.com/openshift/cluster-storage-operator/blob/master/manifests/10_deployment.yaml#L151) but not in hypershift. I'll close this PR as erroneous.

@mpatlasov

mpatlasov commented Sep 19, 2023

Copy link
Copy Markdown
Contributor Author

The path /var/run/secrets/serving-cert is not mounted in hypershift version of Deployment. Hence those extra args are not required.

@mpatlasov mpatlasov closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@csrwng csrwng csrwng left review comments

Assignees

@csrwng csrwng

@dobsonj dobsonj

Labels

area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mpatlasov @openshift-ci-robot @dobsonj @csrwng