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

Bump operator-sdk version to v1.13 #444

Merged
merged 2 commits into from
Mar 29, 2022

Conversation

AKamyshnikova
Copy link
Contributor

Refactor structure and rewrite e2e test acconding to
https://sdk.operatorframework.io/docs/building-operators/golang/migration

Change log description

Refactor structure
Adopt kustomize usage
Rework e2e tests

Purpose of the change

Fixes: #443

@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #444 (b733cce) into master (635edd5) will increase coverage by 1.09%.
The diff coverage is 77.47%.

@@            Coverage Diff             @@
##           master     #444      +/-   ##
==========================================
+ Coverage   84.11%   85.20%   +1.09%     
==========================================
  Files          12       12              
  Lines        1643     1602      -41     
==========================================
- Hits         1382     1365      -17     
+ Misses        177      153      -24     
  Partials       84       84              
Impacted Files Coverage Δ
api/v1beta1/status.go 100.00% <ø> (ø)
api/v1beta1/zookeepercluster_types.go 98.79% <ø> (ø)
api/v1beta1/zz_generated.deepcopy.go 97.91% <ø> (ø)
pkg/utils/zookeeper_util.go 100.00% <ø> (ø)
pkg/yamlexporter/exportutil.go 61.90% <ø> (ø)
pkg/zk/generators.go 100.00% <ø> (ø)
pkg/zk/zookeeper_client.go 83.87% <ø> (ø)
controllers/zookeepercluster_controller.go 66.06% <77.47%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 635edd5...b733cce. Read the comment docs.

@AKamyshnikova AKamyshnikova force-pushed the bump-operator-sdk branch 4 times, most recently from 84b503e to cc3a65d Compare March 2, 2022 09:12
@anishakj
Copy link
Contributor

anishakj commented Mar 2, 2022

@AKamyshnikova Thanks for creating this PR. Have you verified zookeeper-operator upgrade works fine or is there any pre-requisites before doing upgrade?

@AKamyshnikova
Copy link
Contributor Author

@anishakj Yes, I checked zookeeper-operator upgrade - no additional steps required. It is usual upgrade from users point of view.

@anishakj
Copy link
Contributor

anishakj commented Mar 3, 2022

@anishakj Yes, I checked zookeeper-operator upgrade - no additional steps required. It is usual upgrade from users point of view.

Have you tried upgrading using helm as well. Could you please share the output if possible?

@AKamyshnikova
Copy link
Contributor Author

@anishakj I checked this now, works fine. Pasting output:

ubuntu@ataraday-bionic:~/zookeeper-operator$ helm install  zookeeper-operator pravega/zookeeper-operator --version=0.2.13
WARNING: Kubernetes configuration file is group-readable. This is insecure. Location: /home/ubuntu/kubeconf
WARNING: Kubernetes configuration file is world-readable. This is insecure. Location: /home/ubuntu/kubeconf
NAME: zookeeper-operator
LAST DEPLOYED: Thu Mar  3 10:25:54 2022
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl apply -f config/samples/pravega/zookeeper_v1beta1_zookeepercluster_cr.yaml
zookeepercluster.zookeeper.pravega.io/zookeeper created
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl get pods
NAME                                 READY   STATUS    RESTARTS   AGE
zookeeper-0                          0/1     Running   0          13s
zookeeper-operator-8d49bbb6f-cj465   1/1     Running   0          74s
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl get pods
NAME                                 READY   STATUS    RESTARTS   AGE
zookeeper-0                          1/1     Running   0          8m23s
zookeeper-1                          1/1     Running   2          8m6s
zookeeper-2                          1/1     Running   0          7m31s
zookeeper-operator-8d49bbb6f-cj465   1/1     Running   0          9m24s
ubuntu@ataraday-bionic:~/zookeeper-operator$ helm upgrade zookeeper-operator ./charts/zookeeper-operator/
WARNING: Kubernetes configuration file is group-readable. This is insecure. Location: /home/ubuntu/kubeconf
WARNING: Kubernetes configuration file is world-readable. This is insecure. Location: /home/ubuntu/kubeconf
Release "zookeeper-operator" has been upgraded. Happy Helming!
NAME: zookeeper-operator
LAST DEPLOYED: Thu Mar  3 10:40:41 2022
NAMESPACE: default    
STATUS: deployed                                                              
REVISION: 2                               
TEST SUITE: None
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl get pods                               
NAME                                  READY   STATUS    RESTARTS   AGE
zookeeper-0                           1/1     Running   0          14m
zookeeper-1                           1/1     Running   2          14m
zookeeper-2                           1/1     Running   0          13m
zookeeper-operator-8587885857-86jcl   1/1     Running   0          47s                                                                                   
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl describe pod zookeeper-operator-8587885857-86jcl                                                           
Name:         zookeeper-operator-8587885857-86jcl                                                                                                               
Namespace:    default                                                                                                                                           
Priority:     0                                                                                                                                                 
Node:         at-ws-sob242d6gtkm-0-jw4cijbdezzc-server-shq62fbmeyt4/10.10.0.49                                                                                  
Start Time:   Thu, 03 Mar 2022 10:40:47 +0000                                                                                                                   
Labels:       component=zookeeper-operator                                                                                                                      
              name=zookeeper-operator
              pod-template-hash=8587885857       
Annotations:  kubernetes.io/psp: privileged                               
              seccomp.security.alpha.kubernetes.io/pod: docker/default      
Status:       Running
IP:           192.168.250.29                         
IPs:                                                 
  IP:           192.168.250.29                                                                                                                                  Controlled By:  ReplicaSet/zookeeper-operator-8587885857
Containers:                                                                                      
  zookeeper-operator:                                                                                                        
    Container ID:  docker://88b0d2f7aee5ac47db9e1948238f78eb9cd6663390045e362ba52d2fbc437c14
    Image:         docker.io/ataraday/controller:check                            
    Image ID:      docker-pullable://ataraday/controller@sha256:d7389a084ec9694aa80fbed92bbcd7c4297b39354d3f5077f55b442324e2f748
    Port:          <none>
    Host Port:     <none>
    Command:
      zookeeper-operator
    Args:
      --metrics-bind-address=127.0.0.1:6000
    State:          Running
      Started:      Thu, 03 Mar 2022 10:40:51 +0000
    Ready:          True
    Restart Count:  0
    Environment:
      WATCH_NAMESPACE:
      POD_NAME:         zookeeper-operator-8587885857-86jcl (v1:metadata.name)
      OPERATOR_NAME:    zookeeper-operator
    Mounts:
      /var/run/secrets/kubernetes.io/serviceaccount from zookeeper-operator-token-jtd7d (ro)
Conditions:
  Type              Status
  Initialized       True
  Ready             True
  ContainersReady   True
  PodScheduled      True
Volumes:
  zookeeper-operator-token-jtd7d:
    Type:        Secret (a volume populated by a Secret)
    SecretName:  zookeeper-operator-token-jtd7d
    Optional:    false
QoS Class:       BestEffort
Node-Selectors:  <none>
Tolerations:     com.docker.ucp.manager op=Exists
                 node.kubernetes.io/not-ready:NoExecute op=Exists for 300s
                 node.kubernetes.io/unreachable:NoExecute op=Exists for 300s
Events:
  Type    Reason     Age   From               Message
  ----    ------     ----  ----               -------
  Normal  Scheduled  74s   default-scheduler  Successfully assigned default/zookeeper-operator-8587885857-86jcl to at-ws-sob242d6gtkm-0-jw4cijbdezzc-server-shq6
2fbmeyt4
  Normal  Pulling    73s   kubelet            Pulling image "docker.io/ataraday/controller:check"
  Normal  Pulled     70s   kubelet            Successfully pulled image "docker.io/ataraday/controller:check" in 2.731768656s
  Normal  Created    70s   kubelet            Created container zookeeper-operator
  Normal  Started    70s   kubelet            Started container zookeeper-operator
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl get pods
NAME                                  READY   STATUS    RESTARTS   AGE
zookeeper-0                           1/1     Running   0          16m
zookeeper-1                           1/1     Running   2          15m
zookeeper-2                           1/1     Running   0          15m
zookeeper-operator-8587885857-86jcl   1/1     Running   0          2m27s
ubuntu@ataraday-bionic:~/zookeeper-operator$ kubectl delete -f config/samples/pravega/zookeeper_v1beta1_zookeepercluster_cr.yaml
zookeepercluster.zookeeper.pravega.io "zookeeper" deleted
ubuntu@ataraday-bionic:~/zookeeper-operator$ RUN_LOCAL=false go test -failfast -v -timeout 1h ./test/e2e...
=== RUN   TestAPIs                                                                                                                                     [27/1899]
Running Suite: Controller e2e Suite                                   
===================================                              
Random Seed: 1646306521                   
Will run 7 of 7 specs                                              
                              
I0303 11:22:03.179233     606 request.go:655] Throttling request took 1.044496901s, request: GET:https://172.16.241.90:6443/apis/compose.docker.com/v1beta1?time
out=32s                  
• [SLOW TEST:287.716 seconds]                                
Basic test controller          
/home/ubuntu/zookeeper-operator/test/e2e/basic_test.go:20      
  Check create/delete operations                           
  /home/ubuntu/zookeeper-operator/test/e2e/basic_test.go:21      
    should create and recreate a Zookeeper cluster with the same name
    /home/ubuntu/zookeeper-operator/test/e2e/basic_test.go:22
------------------------------
• [SLOW TEST:368.835 seconds]                                       
Delete pods in zk clusters                            
/home/ubuntu/zookeeper-operator/test/e2e/pod_deletion_test.go:21      
  Delete pods, check that pods get recreated                   
  /home/ubuntu/zookeeper-operator/test/e2e/pod_deletion_test.go:22      
    should keep number of replicas consistent
    /home/ubuntu/zookeeper-operator/test/e2e/pod_deletion_test.go:23
------------------------------      
• [SLOW TEST:659.327 seconds]                          
Perform rolling restart on zk cluster
/home/ubuntu/zookeeper-operator/test/e2e/rolling_restart_test.go:20
  Check rolling restart operation                                
  /home/ubuntu/zookeeper-operator/test/e2e/rolling_restart_test.go:21
    should perform rolling restart
    /home/ubuntu/zookeeper-operator/test/e2e/rolling_restart_test.go:22
------------------------------
• [SLOW TEST:332.037 seconds]
Perform zk cluster upgrade
/home/ubuntu/zookeeper-operator/test/e2e/upgrade_test.go:20
  Check zk cluster upgrade operation
  /home/ubuntu/zookeeper-operator/test/e2e/upgrade_test.go:21
    should update spec image version
    /home/ubuntu/zookeeper-operator/test/e2e/upgrade_test.go:22
------------------------------
• [SLOW TEST:1012.964 seconds]
Operations with multiple cluster
/home/ubuntu/zookeeper-operator/test/e2e/multiple_zk_test.go:22
  Perform create, update, delete and recreate operations on 3 clusters
  /home/ubuntu/zookeeper-operator/test/e2e/multiple_zk_test.go:23
    should create cluster several clusters
    /home/ubuntu/zookeeper-operator/test/e2e/multiple_zk_test.go:24
------------------------------
• [SLOW TEST:182.364 seconds]
Ephemeral test controller
/home/ubuntu/zookeeper-operator/test/e2e/ephemeral_test.go:19
  Check create/scale operations
  /home/ubuntu/zookeeper-operator/test/e2e/ephemeral_test.go:20
    should create and scale up and down a Zookeeper cluster
    /home/ubuntu/zookeeper-operator/test/e2e/ephemeral_test.go:21
------------------------------
• [SLOW TEST:121.463 seconds]
Image pull secret check
/home/ubuntu/zookeeper-operator/test/e2e/image_pullsecret_test.go:21
  Check create cluster with specified ImagePullSecrets
  /home/ubuntu/zookeeper-operator/test/e2e/image_pullsecret_test.go:22
    should create cluster with specified ImagePullSecrets specs
    /home/ubuntu/zookeeper-operator/test/e2e/image_pullsecret_test.go:23
------------------------------

Ran 7 of 7 Specs in 2973.559 seconds
SUCCESS! -- 7 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestAPIs (2973.56s)
PASS
ok      github.com/pravega/zookeeper-operator/test/e2e  2973.591s

If some other checks required, let me know.

@AKamyshnikova AKamyshnikova force-pushed the bump-operator-sdk branch 2 times, most recently from 90ea396 to 718c91f Compare March 3, 2022 13:05
@anishakj
Copy link
Contributor

@AKamyshnikova there are some references to cmd/manager/main.go in Makefile. Could you please remove those. Also seeing that scale-test.go is removed from e2e. Could you please add that test as well

@AKamyshnikova
Copy link
Contributor Author

@anishakj Drop of test was not intentional, I restored it.

@anishakj anishakj requested a review from nishant-yt March 28, 2022 14:30
@anishakj
Copy link
Contributor

anishakj commented Mar 28, 2022

@AKamyshnikova make bundle execution is giving me the below error . Is it working for you? Could you please check

FATA[0000] error reading configuration: unable to load the configuration: unable to create config for version "3-alpha": version 3-alpha is not supported 
Makefile:240: recipe for target 'bundle' failed

Also, config/crd/base has 2 CRD yaml files. Can we keep only one?

@AKamyshnikova
Copy link
Contributor Author

@anishakj Sorry it was autogenerated I didn't check this as I added deploy command for test execution and used it.
I fixed it, although I think it is not necessary to use it - it is just of of the option to run operator
https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/#run-the-operator
controller-gen generates crd with name config/crd/bases/zookeeper.pravega.io_zookeeperclusters.yaml if it is fine to drop old name I will drop it. (I checked and there is no way to specify the name, there is an option to modify existing file, but new one is always going to be created)

@anishakj
Copy link
Contributor

anishakj commented Mar 29, 2022

Also, config/crd/base has 2 CRD yaml files. Can we keep only one?

@anishakj Sorry it was autogenerated I didn't check this as I added deploy command for test execution and used it. I fixed it, although I think it is not necessary to use it - it is just of of the option to run operator https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/#run-the-operator controller-gen generates crd with name config/crd/bases/zookeeper.pravega.io_zookeeperclusters.yaml if it is fine to drop old name I will drop it. (I checked and there is no way to specify the name, there is an option to modify existing file, but new one is always going to be created)

In that case can we remove make bundle as it is not used anywhere. Also I could see the CRD generated is not used in https://github.com/AKamyshnikova/zookeeper-operator/blob/bump-operator-sdk/config/crd/kustomization.yaml#L5. Is the old crd is used for end to end tests. You can remove the one with crd.yaml from bases folder

@AKamyshnikova
Copy link
Contributor Author

@anishakj I dropped bundle from Makefile, but I updated PROJECT file anyway, so generation through operator-sdk commands, won't give an error.

@anishakj
Copy link
Contributor

@AKamyshnikova Could you please check why e2e is failing now?

Refactor structure and rewrite e2e test acconding to
https://sdk.operatorframework.io/docs/building-operators/golang/migration

Fixes: pravega#443

Signed-off-by: akamyshnikova <[email protected]>
@AKamyshnikova
Copy link
Contributor Author

@anishakj I fixed config/crd/kustomization.yaml with latest commit, missed bases folder in path for crd.

@amuraru
Copy link
Contributor

amuraru commented Mar 29, 2022

looks good to me

Copy link
Contributor

@anishakj anishakj left a comment

Choose a reason for hiding this comment

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

LGTM

@anishakj anishakj merged commit 610f8b2 into pravega:master Mar 29, 2022
@anishakj
Copy link
Contributor

anishakj commented Apr 6, 2022

@AKamyshnikova Do you have any idea why the logs in https://github.com/pravega/zookeeper-operator/blob/master/pkg/test/e2e/e2eutil/zookeepercluster_util.go are not logged while e2e is running

@AKamyshnikova
Copy link
Contributor Author

@anishakj I will look into this, I guess there is a problem with log setup
https://github.com/pravega/zookeeper-operator/blob/master/test/e2e/suite_test.go#L50

@anishakj
Copy link
Contributor

anishakj commented Apr 8, 2022

@AKamyshnikova have one more query, seeing that crds generated by make generate and make manifests are different. I feel both should be same. Do you know why it is different?

@AKamyshnikova
Copy link
Contributor Author

AKamyshnikova commented Apr 8, 2022

@anishakj make manifests was autogenerated by operator-sdk. The difference is be caused by passing options $(CRD_OPTIONS) rbac:roleName=manager-role webhook to controller-gen. If these options required (they were not used previously) make generate can call make manifests instead of direct calls for controller-gen.

@anishakj
Copy link
Contributor

anishakj commented Apr 8, 2022

@AKamyshnikova It is better to keep CRDs in helm charts consistent with config/bases/crd.

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.

Use operator-sdk 1.13.0 version
3 participants