Skip to content

Conversation

@xiancao
Copy link
Contributor

@xiancao xiancao commented Jan 25, 2022

Currently TwoDomainLoadBalancer class has all the LoadBalancer usecase in one class and the Test methods has unnecessary dependencies on each other. In order to avoid dependency,
Create separate class for each LB. Move the Test(1) and Test(2) to a separate class. For each LoadBalancer class beforeAll() create two domains and install the LoadBalancer, then all tests are independent.

Jenkins job:
https://build.weblogick8s.org:8443/view/all/job/weblogic-kubernetes-operator-kind-new/8200/
https://build.weblogick8s.org:8443/view/all/job/weblogic-kubernetes-operator-kind-new/8201/

Copy link
Member

@anpanigr anpanigr left a comment

Choose a reason for hiding this comment

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

Should we rename the test classes to reflect Two Domains Configuration
ItLbTraefik.java --> ItLbTwoDomainsTraefik.java
Same for all new classes.

Since LoadBalancer test can not be run inside OKD, do we still need to provide OKD compatibility ?

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assumptions.assumeFalse;

/**
Copy link
Member

Choose a reason for hiding this comment

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

Modify the description specific to Traefik LoadBalancer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

Choose a reason for hiding this comment

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

Test a single operator can manage multiple WebLogic domains with as single Traefik fronted loadbalancer -->
Test a single operator can manage multiple WebLogic domains with a single Traefik fronted loadbalancer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@maggiehe00 maggiehe00 left a comment

Choose a reason for hiding this comment

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

Agree with @anpanigr right now we don't run external LB on OKD so no corresponding OKD code are needed in It(LB) test classes.

Copy link
Member

@anpanigr anpanigr left a comment

Choose a reason for hiding this comment

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

Can u add a - before the index in domain id as follow ...

domainUids.add("wls-nginx-domain" + i); to domainUids.add("wls-nginx-domain-" + i);
domainUids.add("wls-apache-domain" + i); to domainUids.add("wls-apache-domain-" + i);
domainUids.add("wls-traefik-domain" + i); to domainUids.add("wls-traefik-domain-" + i);

for better visibility during triaging

Copy link
Member

@anpanigr anpanigr left a comment

Choose a reason for hiding this comment

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

Modify the script and pom files which refers to ItTwoDomainsLoadBalancers
I found one file oketest.sh

@xiancao
Copy link
Contributor Author

xiancao commented Jan 27, 2022

Can u add a - before the index in domain id as follow ...

domainUids.add("wls-nginx-domain" + i); to domainUids.add("wls-nginx-domain-" + i); domainUids.add("wls-apache-domain" + i); to domainUids.add("wls-apache-domain-" + i); domainUids.add("wls-traefik-domain" + i); to domainUids.add("wls-traefik-domain-" + i);

for better visibility during triaging

fixed

@xiancao
Copy link
Contributor Author

xiancao commented Jan 27, 2022

Modify the script and pom files which refers to ItTwoDomainsLoadBalancers
I found one file oketest.sh

fixed

@xiancao xiancao requested a review from anpanigr January 27, 2022 18:54
Copy link
Member

@anpanigr anpanigr left a comment

Choose a reason for hiding this comment

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

LGTM

@robertpatrick robertpatrick merged commit f415f6f into main Jan 28, 2022
xiancao added a commit that referenced this pull request Jan 28, 2022
…arate LoadBalancers (#2731)

* wip

* re-org loadbalancer tests

* re-org loadbalancer tests

* fix expected error msg

* fix copyright

* address Pani and Maggies's comments

* address Pani's comments

* address Pani's comments
@xiancao xiancao deleted the xc-92759 branch January 28, 2022 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants