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

Pravega Operator API Resource Hook. #424

Closed
wants to merge 11 commits into from
Closed

Pravega Operator API Resource Hook. #424

wants to merge 11 commits into from

Conversation

co-jo
Copy link
Member

@co-jo co-jo commented Jul 15, 2020

Change log description

  • Adds a manifest which defines a post-install/upgrade hook to make sure the pravegacluster resource exists.

Purpose of the change

  • If one is unlucky with the timings when install the pravega-operator chart, directly followed by the pravega chart, the api-resource PravegaCluster may not exist by the time the pravega chart tries to create a PravegaCluster.

What the code does

  • See above.

How to verify it

Run many install/uninstall cycles of the mentioned charts -- see that the pravega chart does not start before its resource dependency is registered.

co-jo and others added 2 commits July 15, 2020 12:20
@SrishT
Copy link
Contributor

SrishT commented Jul 16, 2020

@co-jo although this is a great add-on for our charts, it isn't failing in the negative case as it should.
Like I gave an incorrect operator image to fail the deployment

$ kubectl get pods
NAME                                   READY   STATUS             RESTARTS   AGE
bookkeeper-bookie-0                    1/1     Running            0          7d12h
bookkeeper-bookie-1                    1/1     Running            0          7d12h
bookkeeper-bookie-2                    1/1     Running            0          7d12h
bookkeeper-operator-6b8f99d879-v4fh4   1/1     Running            0          7d12h
nfs-server-provisioner-1594131302-0    1/1     Running            0          8d
pravega-operator-5c5776fb84-njhd4      0/1     ImagePullBackOff   0          20m
zookeeper-0                            1/1     Running            0          7d12h
zookeeper-1                            1/1     Running            0          7d12h
zookeeper-2                            1/1     Running            0          7d12h
zookeeper-operator-7b45886df9-kkz56    1/1     Running            0          7d12h

And kubectl api-resources | grep pravegaclusters returns no response
But even then helm installation does not reflect this failure and shows that the resource is successfully deployed

$ helm install pravega-operator charts/pravega-operator/
NAME: pravega-operator
LAST DEPLOYED: Thu Jul 16 01:57:23 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None

$ helm ls
NAME                             	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART                       	APP VERSION   
pravega-operator                 	default  	1       	2020-07-16 03:26:57.77338856 +0530 IST 	deployed	pravega-operator-0.5.1      	0.5.1      

Could u look into the negative test case for all 3 operators?

@co-jo
Copy link
Member Author

co-jo commented Jul 16, 2020

@co-jo although this is a great add-on for our charts, it isn't failing in the negative case as it should.
Like I gave an incorrect operator image to fail the deployment

$ kubectl get pods
NAME                                   READY   STATUS             RESTARTS   AGE
bookkeeper-bookie-0                    1/1     Running            0          7d12h
bookkeeper-bookie-1                    1/1     Running            0          7d12h
bookkeeper-bookie-2                    1/1     Running            0          7d12h
bookkeeper-operator-6b8f99d879-v4fh4   1/1     Running            0          7d12h
nfs-server-provisioner-1594131302-0    1/1     Running            0          8d
pravega-operator-5c5776fb84-njhd4      0/1     ImagePullBackOff   0          20m
zookeeper-0                            1/1     Running            0          7d12h
zookeeper-1                            1/1     Running            0          7d12h
zookeeper-2                            1/1     Running            0          7d12h
zookeeper-operator-7b45886df9-kkz56    1/1     Running            0          7d12h

And kubectl api-resources | grep pravegaclusters returns no response
But even then helm installation does not reflect this failure and shows that the resource is successfully deployed

$ helm install pravega-operator charts/pravega-operator/
NAME: pravega-operator
LAST DEPLOYED: Thu Jul 16 01:57:23 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None

$ helm ls
NAME                             	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART                       	APP VERSION   
pravega-operator                 	default  	1       	2020-07-16 03:26:57.77338856 +0530 IST 	deployed	pravega-operator-0.5.1      	0.5.1      

Could u look into the negative test case for all 3 operators?

Sure, thanks for the input!

Edit: I believe the result you mentioned is unrelated to my changes. I tested the situation you described using the v0.5.1-rc0 release and get the same results. I suppose I could work to fix that problem, but that would be a separate issue -- unrelated to the motivation for these 3 PRs.

colin@unix:~/tmp$ cd charts/pravega-operator && git status
HEAD detached at v0.5.1-rc0
nothing to commit, working tree clean
colin@unix:~/tmp/charts/pravega-operator$ cd ~/tmp
colin@unix:~/tmp$ helm install pravega-operator charts/pravega-operator/charts/pravega-operator \
>         --set webhookCert.crt=$(cat crt.txt)\
>         --set image.tag="1234"
NAME: pravega-operator
LAST DEPLOYED: Thu Jul 16 13:13:09 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
colin@unix:~/tmp$ helm list
NAME                    NAMESPACE       REVISION        UPDATED                                 STATUS          CHART                           APP VERSION
bookkeeper-cluster      default         1               2020-07-16 12:55:52.02599322 -0700 PDT  deployed        bookkeeper-0.7.0                0.7.0
bookkeeper-operator     default         1               2020-07-16 12:55:46.318653036 -0700 PDT deployed        bookkeeper-operator-0.1.2       0.1.2
nfs-client-provisioner  default         1               2020-07-16 12:53:27.885740176 -0700 PDT deployed        nfs-client-provisioner-1.2.8    3.1.0
pravega-operator        default         1               2020-07-16 13:13:09.816741198 -0700 PDT deployed        pravega-operator-0.5.1          0.5.1
zookeeper-cluster       default         1               2020-07-16 12:53:42.911246015 -0700 PDT deployed        zookeeper-0.1.0
zookeeper-operator      default         1               2020-07-16 12:53:35.437655779 -0700 PDT deployed        zookeeper-operator-0.1.0
colin@unix:~/tmp$ kubectl get pods
NAME                                      READY   STATUS         RESTARTS   AGE
bookkeeper-cluster-bookie-0               1/1     Running        0          17m
bookkeeper-cluster-bookie-1               1/1     Running        0          17m
bookkeeper-cluster-bookie-2               1/1     Running        0          17m
bookkeeper-operator-96596cdf-x9zzs        1/1     Running        0          17m
nfs-client-provisioner-795dcfb54d-csn2w   1/1     Running        0          20m
pravega-operator-6f9445b5c4-mq58f         0/1     ErrImagePull   0          20s
zookeeper-cluster-0                       1/1     Running        0          19m
zookeeper-cluster-1                       1/1     Running        0          19m
zookeeper-cluster-2                       1/1     Running        0          18m
zookeeper-operator-76bd88d64f-v7t2q       1/1     Running        0          19m

@co-jo co-jo changed the title Pravega Operator API Resource Hook Pravega Operator API Resource Hook. Jul 20, 2020
@co-jo co-jo closed this Jul 20, 2020
@co-jo
Copy link
Member Author

co-jo commented Jul 20, 2020

Closed (and replaced with #427) due to complication with fixing the DCO check.

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.

2 participants