operator-sdk add api for Ansible/Helm operators. #2703
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
To test out, please run the command in an existing project scaffold. |
asmacdo
reviewed
Mar 23, 2020
asmacdo
reviewed
Mar 23, 2020
asmacdo
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
joelanford
reviewed
Mar 23, 2020
jmrodri
changed the title
operator-sdk add api for Ansible/Helm operators.
openshift-ci-robot
added
the
needs-rebase
bharathi-tenneti
force-pushed
the
ansible/helm-addapi
branch
from
b5610e4 to
1ece2b8
Compare
openshift-ci-robot
removed
the
needs-rebase
| @@ -309,6 +309,8 @@ kubectl delete -f deploy/role.yaml | |||
| kubectl delete -f deploy/service_account.yaml | |||
| kubectl delete -f deploy/crds/example.com_nginxes_crd.yaml | |||
| ``` | |||
| **NOTE** Additional CR/CRD's can be added to the project by running, for example, the command :`operator-sdk add api --api-version=cache.example.com/v1alpha1 --kind=AppService` | |||
| For more information, refer [cli][addcli] doc. | |||
|
|
|||
There was a problem hiding this comment.
Same issue faced with the watches for helm as well. See:
The command used over the Helm Mencached sample was: $ operator-sdk add api --api-version=cache.example.com/v1alpha1 --kind=AppService
There was a problem hiding this comment.
IMO, this one is non-blocking. This doesn't change the behavior at all. Reordering of fields is a minor nuisance. The added watchDependentResources is a quirk of YAML marshalling due to the fact that we set watchDependentResources = true as a default when we unmarshal.
joelanford
reviewed
Apr 9, 2020
internal/scaffold/helm/watches.go
Outdated
| } | ||
| newWatch := watches.Watch{ | ||
| GroupVersionKind: gvk, | ||
| ChartDir: HelmChartsDir + "/" + r.LowerKind, |
There was a problem hiding this comment.
To fix the issue @camilamacedo86 noted about the mismatched chart directory name, we need to pass chart to this function and use that to set ChartDir
There was a problem hiding this comment.
@joelanford I am guessing this is an existing bug as well if I understand
Currently, when creating new watch, as you see below s.Resource.LowerKind is being set. Or, is it only conditional. Could get clarification here.
type WatchesYAML struct {
input.Input
Resource *scaffold.Resource
HelmChartsDir string
ChartName string
}
// GetInput gets the scaffold execution input
func (s *WatchesYAML) GetInput() (input.Input, error) {
if s.Path == "" {
s.Path = WatchesYamlFile
}
s.HelmChartsDir = HelmChartsDir
s.TemplateBody = watchesYAMLTmpl
if s.ChartName == "" {
s.ChartName = s.Resource.LowerKind
}
return s.Input, nil
}There was a problem hiding this comment.
That's only if ChartName is empty. We set it here.
joelanford
reviewed
Apr 14, 2020
internal/scaffold/role.go
Outdated
Comment on lines
149
to
150
| // MergeHelmRoleForResource merges incoming new API resource rules with existing deploy/role.yaml | ||
| func MergeHelmRoleForResource(r *Resource, absProjectPath string, roleScaffold Role) error { |
There was a problem hiding this comment.
Is this specific to Helm? This can probably be renamed to MergeRoleForResource, right?
If it is specfic to Helm, we should move it to somewhere in internal/scaffold/helm.
joelanford
reviewed
Apr 14, 2020
bharathi-tenneti
force-pushed
the
ansible/helm-addapi
branch
from
346228b to
b0741cf
Compare
openshift-ci-robot
added
the
needs-rebase
bharathi-tenneti
force-pushed
the
ansible/helm-addapi
branch
from
b0741cf to
8657a5f
Compare
openshift-ci-robot
removed
the
needs-rebase
|
Ansible operator testing results for $ /Users/btenneti/go/bin/operator-sdk new ansible-demo --api-version=cache.example.com/v1alpha1 --kind=Memcached --type=ansible
INFO[0000] Creating new Ansible operator 'ansible-demo'.
INFO[0000] Created deploy/service_account.yaml
INFO[0000] Created deploy/role.yaml
INFO[0000] Created deploy/role_binding.yaml
INFO[0000] Created deploy/crds/cache.example.com_v1alpha1_memcached_cr.yaml
INFO[0000] Created build/Dockerfile
INFO[0000] Created roles/memcached/README.md
INFO[0000] Created roles/memcached/meta/main.yml
INFO[0000] Created roles/memcached/files/.placeholder
INFO[0000] Created roles/memcached/templates/.placeholder
INFO[0000] Created roles/memcached/vars/main.yml
INFO[0000] Created molecule/test-local/converge.yml
INFO[0000] Created roles/memcached/defaults/main.yml
INFO[0000] Created roles/memcached/tasks/main.yml
INFO[0000] Created molecule/default/molecule.yml
INFO[0000] Created molecule/default/prepare.yml
INFO[0000] Created molecule/default/converge.yml
INFO[0000] Created molecule/default/verify.yml
INFO[0000] Created roles/memcached/handlers/main.yml
INFO[0000] Created watches.yaml
INFO[0000] Created deploy/operator.yaml
INFO[0000] Created .travis.yml
INFO[0000] Created requirements.yml
INFO[0000] Created molecule/test-local/molecule.yml
INFO[0000] Created molecule/test-local/prepare.yml
INFO[0000] Created molecule/test-local/verify.yml
INFO[0000] Created molecule/cluster/molecule.yml
INFO[0000] Created molecule/cluster/create.yml
INFO[0000] Created molecule/cluster/prepare.yml
INFO[0000] Created molecule/cluster/converge.yml
INFO[0000] Created molecule/cluster/verify.yml
INFO[0000] Created molecule/cluster/destroy.yml
INFO[0000] Created molecule/templates/operator.yaml.j2
INFO[0000] Generated CustomResourceDefinition manifests.
INFO[0000] Project creation complete. apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
creationTimestamp: null
name: ansible-demo
rules:
- apiGroups:
- ""
resources:
- pods
- services
- services/finalizers
- endpoints
- persistentvolumeclaims
- events
- configmaps
- secrets
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- apps
resources:
- deployments
- daemonsets
- replicasets
- statefulsets
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- monitoring.coreos.com
resources:
- servicemonitors
verbs:
- get
- create
- apiGroups:
- apps
resourceNames:
- ansible-demo
resources:
- deployments/finalizers
verbs:
- update
- apiGroups:
- ""
resources:
- pods
verbs:
- get
- apiGroups:
- apps
resources:
- replicasets
- deployments
verbs:
- get
- apiGroups:
- cache.example.com
resources:
- '*'
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- app.example.com
resources:
- '*'
verbs:
- create
- delete
- get
- list
- patch
- update
- watch |
|
Helm Operator testing results for $ /Users/btenneti/go/bin/operator-sdk new helm-demo --helm-chart=stable/nginx-ingress --type=helm
INFO[0000] Creating new Helm operator 'helm-demo'.
INFO[0001] Created helm-charts/nginx-ingress
INFO[0001] Generating RBAC rules
INFO[0001] Scaffolding ClusterRole and ClusterRolebinding for cluster scoped resources in the helm chart
WARN[0001] The RBAC rules generated in deploy/role.yaml are based on the chart's default manifest. Some rules may be missing for resources that are only enabled with custom values, and some existing rules may be overly broad. Double check the rules generated in deploy/role.yaml to ensure they meet the operator's permission requirements.
INFO[0001] Created build/Dockerfile
INFO[0001] Created watches.yaml
INFO[0001] Created deploy/service_account.yaml
INFO[0001] Created deploy/role.yaml
INFO[0001] Created deploy/role_binding.yaml
INFO[0001] Created deploy/operator.yaml
INFO[0001] Created deploy/crds/charts.helm.k8s.io_v1alpha1_nginxingress_cr.yaml
INFO[0001] Generated CustomResourceDefinition manifests.
INFO[0001] Project creation complete. apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
creationTimestamp: null
name: helm-demo
rules:
- apiGroups:
- ""
resources:
- namespaces
verbs:
- get
- apiGroups:
- ""
resources:
- configmaps
- secrets
- serviceaccounts
- services
verbs:
- '*'
- apiGroups:
- ""
resources:
- events
verbs:
- create
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterrolebindings
- clusterroles
verbs:
- '*'
- apiGroups:
- apps
resources:
- deployments
verbs:
- '*'
- apiGroups:
- ""
resources:
- serviceaccounts
- services
verbs:
- '*'
- apiGroups:
- rbac.authorization.k8s.io
resources:
- rolebindings
- roles
verbs:
- '*'
- apiGroups:
- monitoring.coreos.com
resources:
- servicemonitors
verbs:
- get
- create
- apiGroups:
- apps
resourceNames:
- helm-demo
resources:
- deployments/finalizers
verbs:
- update
- apiGroups:
- ""
resources:
- pods
verbs:
- get
- apiGroups:
- apps
resources:
- replicasets
- deployments
verbs:
- get
- apiGroups:
- charts.helm.k8s.io
resources:
- '*'
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- cache.example.com
resources:
- '*'
verbs:
- create
- delete
- get
- list
- patch
- update
- watch |
estroz
previously requested changes
Apr 16, 2020
joelanford
reviewed
Apr 16, 2020
bharathi-tenneti
force-pushed
the
ansible/helm-addapi
branch
2 times, most recently
from
392297a to
dbe0a82
Compare
*Added Unit Test cases *Matched doc/cli with website/cli
bharathi-tenneti
force-pushed
the
ansible/helm-addapi
branch
from
df5939f to
315697a
Compare
|
/unhold |
openshift-ci-robot
removed
the
do-not-merge/hold
jmrodri
dismissed
estroz’s stale review
@estroz left comments left 2 days ago. Bharathi addressed them all. Dismissing the review in order to merge.
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: Enable Ansible/Helm operator developers to create additional APIs, through SDK CLI, using below command.
Ansible Example:
operator-sdk add api --api-version=example.com/v1alpha1 --kind=MemcachedHelm Example: (chart options being optional)
operator-sdk add api --api-version=example.com/v1alpha1 --kind=Memcached --helm-chart=stable/memcachedMotivation:
As of today, SDK CLI can be used only to add additonal APIs for Go based Operators. Ansible/Helm operator developers are not able to create additonal APIs via CLI, once the original project scaffolds. Today, developers have to manually add necessary files to the project scaffold.
For more information please refer proposal.