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

Incorrect configuration of namespace object for the Red Hat OpenShift Logging Operator #86672

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

prithvipatil97
Copy link
Contributor

@prithvipatil97 prithvipatil97 commented Jan 3, 2025

apiVersion: v1
kind: Namespace
metadata:
  name: openshift-logging 
annotations:                                     <<== Incorrect Indentation
    openshift.io/node-selector: ""
labels:                                          <<== Incorrect Indentation
    openshift.io/cluster-logging: "true"
    openshift.io/cluster-monitoring: "true" 
  • annotations, and labels should be indented under the metadata field and inline with name.
  • Here is the correct configuration of Step5:
apiVersion: v1
kind: Namespace
metadata:
  name: openshift-logging 
  annotations:                                   <<== Correct Indentation
    openshift.io/node-selector: ""
  labels:                                        <<== Correct Indentation
    openshift.io/cluster-logging: "true"
    openshift.io/cluster-monitoring: "true" 
  • We need to perform this change under our standard documentation.
  • Please refer Step1 from the same documentation for reference.

  1. Create a Namespace object for Loki Operator:

Example Namespace object

apiVersion: v1
kind: Namespace
metadata:
  name: openshift-operators-redhat 
  annotations:
    openshift.io/node-selector: ""
  labels:
    openshift.io/cluster-monitoring: "true" 

Version(s):

RHOCP 4.14+

Issue:

https://issues.redhat.com/browse/OBSDOCS-1591

Link to docs preview:

https://86672--ocpdocs-pr.netlify.app/openshift-dedicated/latest/observability/logging/cluster-logging-deploying.html

https://86672--ocpdocs-pr.netlify.app/openshift-dedicated/latest/observability/logging/log_storage/installing-log-storage.html

https://86672--ocpdocs-pr.netlify.app/openshift-enterprise/latest/observability/logging/cluster-logging-deploying.html

https://86672--ocpdocs-pr.netlify.app/openshift-enterprise/latest/observability/logging/log_storage/installing-log-storage.html

https://86672--ocpdocs-pr.netlify.app/openshift-rosa/latest/observability/logging/cluster-logging-deploying.html

https://86672--ocpdocs-pr.netlify.app/openshift-rosa/latest/observability/logging/log_storage/installing-log-storage.html

QE review:

  • QE has approved this change.

Additional information:

… Logging Operator

- Incorrect indentation mentioned in the step5 "namespace object for the Red Hat OpenShift Logging Operator"  [2] of "Installing Logging and the Loki Operator using the CLI" documentation  [1].

- Here are the documentation links:
 [1] https://docs.openshift.com/container-platform/4.14/observability/logging/log_storage/installing-log-storage.html#logging-loki-cli-install_installing-log-storage
 
[2] https://docs.openshift.com/container-platform/4.14/observability/logging/log_storage/installing-log-storage.html#logging-loki-cli-install_installing-log-storage:~:text=Create%20a%20namespace%20object%20for%20the%20Red%20Hat%20OpenShift%20Logging%20Operator%3A

- Here is the Step5:
~~~
apiVersion: v1
kind: Namespace
metadata:
  name: openshift-logging
annotations:                        <<== Incorrect Indentation                      
    openshift.io/node-selector: ""
labels:                             <<== Incorrect Indentation
    openshift.io/cluster-logging: "true"
    openshift.io/cluster-monitoring: "true" 
~~~
- `annotations`, and `labels` should be indented under the `metadata` field and inline with `name`.
- Here is the correct configuration of Step5:
~~~

~~~
- We need to perform this change under our standard documentation.
- Please refer Step1 from the same documentation for reference.
--------
1. Create a Namespace object for Loki Operator:
~~~

~~~
--------------
@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 3, 2025
Copy link

openshift-ci bot commented Jan 3, 2025

@prithvipatil97: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@prithvipatil97
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Jan 3, 2025
@xenolinux xenolinux added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jan 3, 2025
Copy link
Contributor

@xenolinux xenolinux left a comment

Choose a reason for hiding this comment

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

LGTM

@xenolinux xenolinux added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Jan 3, 2025
@prithvipatil97
Copy link
Contributor Author

Hello @xenolinux ,
Thank you very much for reviewing this PR and providing approval.

Regards,
Prithviraj Patil

@prithvipatil97
Copy link
Contributor Author

Hello Team,
I need QE approval for this change.

All tests have passed and Peer review is also done.

@anpingli , @kabirbhartiRH , @QiaolingTang , It would be really helpful if someone could please take a look and provide QE approval for this change.

Thanks in advance.

Regards,
Prithviraj Patil

@QiaolingTang
Copy link

LGTM.

@prithvipatil97
Copy link
Contributor Author

Hello @QiaolingTang ,
Thank you very much for providing the QE approval for this change.

Regards,
Prithviraj Patil

@prithvipatil97
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Jan 6, 2025
@xenolinux xenolinux added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Jan 6, 2025
Copy link
Contributor

@xenolinux xenolinux left a comment

Choose a reason for hiding this comment

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

LGTM
merging...

@xenolinux xenolinux merged commit de31b4b into openshift:main Jan 6, 2025
2 checks passed
@xenolinux
Copy link
Contributor

/cherrypick enterprise-4.17

@xenolinux
Copy link
Contributor

/cherrypick enterprise-4.18

@xenolinux
Copy link
Contributor

/cherrypick enterprise-4.14

@xenolinux
Copy link
Contributor

/cherrypick enterprise-4.15

@xenolinux
Copy link
Contributor

/cherrypick enterprise-4.16

@openshift-cherrypick-robot

@xenolinux: new pull request created: #86694

In response to this:

/cherrypick enterprise-4.17

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-sigs/prow repository.

@openshift-cherrypick-robot

@xenolinux: new pull request created: #86695

In response to this:

/cherrypick enterprise-4.18

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-sigs/prow repository.

@openshift-cherrypick-robot

@xenolinux: #86672 failed to apply on top of branch "enterprise-4.14":

Applying: Incorrect configuration of namespace object for the Red Hat OpenShift Logging Operator
Using index info to reconstruct a base tree...
M	modules/logging-loki-cli-install.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/logging-loki-cli-install.adoc
CONFLICT (content): Merge conflict in modules/logging-loki-cli-install.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Incorrect configuration of namespace object for the Red Hat OpenShift Logging Operator

In response to this:

/cherrypick enterprise-4.14

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-sigs/prow repository.

@openshift-cherrypick-robot

@xenolinux: #86672 failed to apply on top of branch "enterprise-4.15":

Applying: Incorrect configuration of namespace object for the Red Hat OpenShift Logging Operator
Using index info to reconstruct a base tree...
M	modules/logging-loki-cli-install.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/logging-loki-cli-install.adoc
CONFLICT (content): Merge conflict in modules/logging-loki-cli-install.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Incorrect configuration of namespace object for the Red Hat OpenShift Logging Operator

In response to this:

/cherrypick enterprise-4.15

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-sigs/prow repository.

@openshift-cherrypick-robot

@xenolinux: new pull request created: #86696

In response to this:

/cherrypick enterprise-4.16

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-sigs/prow repository.

@xenolinux
Copy link
Contributor

xenolinux commented Jan 6, 2025

@prithvipatil97 You can mention Version as 4.14+ or 4.14 and later since this update is applicable till the latest version.

The automated cherry-pick PRs for 4.14 and 4.15 failed, can you please create manual cherry-pick PRs?

@xenolinux xenolinux removed the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Jan 6, 2025
@prithvipatil97
Copy link
Contributor Author

Hello @xenolinux ,
Thank you very much for a merge review.

I can see changes has been successfully applied in the RHOCP 4.16, RHOCP 4.17, and RHOCP 4.18.

The automated cherry-pick PRs for 4.14 and 4.15 failed.

Okay. sure.
With your suggestion, let me create new PR for RHOCP 4.14 and RHOCP 4.15 version.
I will link those PR here, as we have Peer review and QE approval here. So you can merge those new PR directly.

Thank again for your quick assistance.

Regards,
Prithviraj Patil

@prithvipatil97
Copy link
Contributor Author

Hello @xenolinux
Here are the new PR:

Linked to #86705
Linked to #86707

I am adding a merge review label in the above PR.
Kindly take a look and proceed with merging.

Regards,
Prithviraj Patil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.14 branch/enterprise-4.15 branch/enterprise-4.16 branch/enterprise-4.17 branch/enterprise-4.18 peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants