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

fix no route in ocp 311 and errors in log #468

Merged
merged 1 commit into from
Apr 24, 2022

Conversation

zhiweiyin318
Copy link
Contributor

Signed-off-by: Zhiwei Yin zyin@redhat.com

  1. fix issue: https://github.com/stolostron/backlog/issues/21853
    OCP 311 has route, support to get log using route.

  2. fix issue: https://github.com/stolostron/backlog/issues/21554
    work-agent cannot update service if the type of service is changed in OCP 311, and return error
    invalid: spec.ports[0].nodePort: Forbidden: may not be used when type is 'ClusterIP'
    this is an issue in old k8s version, has been fixed in new one. Unable to change service from type=NodePort to type=ClusterIP with kubectl kubernetes/kubectl#221
    the fix in addon is that deploy service until the product claim has value. will not update it in addon manifestwork.

  3. return error for the interface of the imageRegisty Client to help debug.

Signed-off-by: Zhiwei Yin <zyin@redhat.com>
@sonarcloud
Copy link

sonarcloud bot commented Apr 22, 2022

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 1 Code Smell

83.8% 83.8% Coverage
0.0% 0.0% Duplication

@zhiweiyin318
Copy link
Contributor Author

/assign @qiujian16

@@ -1,4 +1,5 @@
---
{{- if not (eq .Values.product "") }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

not quite sure why this is needed, we do not create service when product is empty?

Copy link
Contributor Author

@zhiweiyin318 zhiweiyin318 Apr 22, 2022

Choose a reason for hiding this comment

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

the previous logic is that we create LoadBalancer service firstly and then update the service to ClusterIP when the product claim is OCP added by work-manager addon .
for OCP 311, work-agent will meet error to update the service invalid: spec.ports[0].nodePort: Forbidden: may not be used when type is 'ClusterIP' because this issue kubernetes/kubectl#221 has not fixed in old k8s.
current fix is that to create service until the product claim is appended to the managedCluster. will not need to update the service.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it means we will not create service in *ks clusters since we cannot know the product? is this desired?

Copy link
Contributor Author

@zhiweiyin318 zhiweiyin318 Apr 22, 2022

Choose a reason for hiding this comment

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

no, the product claim will be Other for *ks, not empty.

@zhiweiyin318
Copy link
Contributor Author

/retest

1 similar comment
@zhiweiyin318
Copy link
Contributor Author

/retest

@qiujian16
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Apr 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, zhiweiyin318

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [qiujian16,zhiweiyin318]

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

@openshift-merge-robot openshift-merge-robot merged commit ffc5b5e into stolostron:main Apr 24, 2022
@zhiweiyin318 zhiweiyin318 deleted the add-route branch April 24, 2022 03:21
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.

3 participants