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

Support using built-in resource types for inlined Kubernetes-typed components #4685

Closed
scottkurz opened this issue May 4, 2021 · 10 comments · Fixed by #5107
Closed

Support using built-in resource types for inlined Kubernetes-typed components #4685

scottkurz opened this issue May 4, 2021 · 10 comments · Fixed by #5107
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).

Comments

@scottkurz
Copy link
Contributor

scottkurz commented May 4, 2021

/kind bug

What versions of software are you using?

Operating System: Win10, Linux

Output of odo version: odo v2.1.0 (8479050)

How did you run odo exactly?

Well, I'm not saying I had a compelling business or technical case for doing this, but just to understand the technology, I tried adding some buit-in type defs as inlined kubernetes-typed components, like this:

components:
  - name: myk8deploy
    kubernetes:
      inlined: |
        apiVersion: v1
        kind: Pod
        metadata:
          name: nginx
          labels:
            name: nginx
        spec:
          containers:
          - name: nginx
            image: nginx
            ports:
            - containerPort: 80

(Note there's no reference to this component from any of the commands.)

I get similar results with including an inline Job type.

Actual behavior

In the VSCode debug console I see:

Failed to start component with name "in1". Error: Failed to create the component: failed to create service(s) associated with the component: could not find specified service/custom resource: Pod; please check the "kind" field in the yaml (it's case-sensitive)

and nothing matching the nginx pod in the namespace via kubectl get all.

In the debugger I see we step through:

func GetCSV(client *kclient.Client, crd map[string]interface{}) (string, olm.ClusterServiceVersion, error) {
	cr := crd["kind"].(string)
	csvs, err := client.ListClusterServiceVersions()

fail to find a CSV match for the Pod type, and exit.

Expected behavior

I'd expect this definition to have been passed to "kubectl apply". odo shouldn't only be creating operator-backed resources.

Any logs, error output, etc?

See above

LINK

This could be grouped with this issue: #4159

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label May 4, 2021
@girishramnani
Copy link
Contributor

This is interesting. would this show up in odo list?
I like this feature but how would this integrate with other commands?

@kadel
Copy link
Member

kadel commented May 4, 2021

This is interesting. would this show up in odo list?
I like this feature but how would this integrate with other commands?

No odo won't be showing this.
Odo should just create this, or delete it if gets removed from devfile.yaml

@kadel kadel added area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). labels May 6, 2021
@dharmit dharmit added this to For Consideration in Sprint 201 via automation May 6, 2021
@dharmit dharmit added this to For Consideration in Sprint 202 via automation May 21, 2021
@dharmit dharmit self-assigned this May 24, 2021
@dharmit dharmit removed this from For Consideration in Sprint 201 May 27, 2021
@dharmit
Copy link
Member

dharmit commented May 27, 2021

Odo should just create this, or delete it if gets removed from devfile.yaml

@kadel a couple of questions based on your comment:

  1. So do we want a Pod or any other Kubernetes resource created this way to show up in any of the odo outputs like odo service list, odo list, etc.? I think no.

  2. Do we just want to create and destroy Kubernetes resources mentioned as Kubernetes inlined component? Would it be user's concern to figure where/how to find them on the Kubernetes cluster?

Scope of the issue:

At the moment, odo checks if the Kind mentioned in the spec is provided by any of the ClusterServiceVersions. This limit odo to be able to create only those resources on the cluster which are provided by an Operator. Instead, we could modify the odo could to parse GVK info for the requested resource and use below function: https://github.com/openshift/odo/blob/3196871acd1ee55dcc96d2bf8b1d6bfb247dfc8b/pkg/kclient/kclient.go#L206

@kadel
Copy link
Member

kadel commented May 27, 2021

  1. So do we want a Pod or any other Kubernetes resource created this way to show up in any of the odo outputs like odo service list, odo list, etc.? I think no.

Correct. Let's not show them anywhere. Unless of course, it is Service (Operator CR)

2. Do we just want to create and destroy Kubernetes resources mentioned as Kubernetes inlined component? Would it be user's concern to figure where/how to find them on the Kubernetes cluster?

Yes, to the first question. The resources should be created and deleted on odo push / odo delete.

I don't understand the second question.

At the moment, odo checks if the Kind mentioned in the spec is provided by any of the ClusterServiceVersions. This limit odo to be able to create only those resources on the cluster which are provided by an Operator. Instead, we could modify the odo could to parse GVK info for the requested resource and use below function:

+1

@dharmit dharmit moved this from For Consideration to In Progress in Sprint 202 Jun 10, 2021
@dharmit dharmit added this to For Consideration in Sprint 203 via automation Jun 14, 2021
@dharmit dharmit moved this from For Consideration to To Do in Sprint 203 Jun 16, 2021
@dharmit dharmit removed this from In Progress in Sprint 202 Jun 16, 2021
@dharmit dharmit added this to For Consideration in Sprint 204 via automation Jul 5, 2021
@dharmit dharmit moved this from For Consideration to To Do in Sprint 204 Jul 7, 2021
@dharmit dharmit removed this from To Do in Sprint 203 Jul 12, 2021
@dharmit
Copy link
Member

dharmit commented Jul 12, 2021

I am using the example mentioned in the issue description. As a first step, I'm trying to figure out if the GVK in the specified Kubernetes inlined component is supported by the underlying cluster.

But I'm hitting an issue that, at the moment, I can't figure how to overcome.

PTAL at the WIP code - https://github.com/dharmit/odo/commit/5bb6c313e1490b6ab4f8e9e2aae5a418cfad0a4e.

We're calling below function with the values core, v1, Pod:

supported, _ := client.IsResourceSupported(group, version, kind)

The problem is with the kind being Pod.

All other calls to IsResourceSupported send the Name column equivalent of Kind in kubectl api-resources output. So for Pod, we must send pods. However, I'm not sure how to obtain this value from the cluster yet.

@dharmit dharmit moved this from To Do to In Progress in Sprint 204 Jul 19, 2021
@dharmit
Copy link
Member

dharmit commented Jul 26, 2021

This issue will spill over to Sprint 205. I need to research how to circumvent the issues being faced in the WIP PR.

@dharmit dharmit added this to For Consideration in Sprint 205 via automation Jul 26, 2021
@dharmit dharmit moved this from For Consideration to To Do in Sprint 205 Jul 26, 2021
@dharmit
Copy link
Member

dharmit commented Jul 29, 2021

Current state of things is documented in #4908 (comment).

There are two main issues right now.

  1. odo code expecting only one Pod matching the labels of the odo component. This could also be interpreted as odo expecting one and only one Pod on the cluster for any odo component.
  2. PVC error that I don't know/understand the cause of yet.

To get over the first problem, we could do either of the following:

  1. Make way for odo to expect more than one Pod for a component.
  2. Make sure that the same labels aren't set on the Kubernetes resources (Pod, Deployment, Service, Secret, anything else) created by a user by modifying the devfile by hands.
  3. Don't set any labels on such Kubernetes resources which are not created from Operators. Only set the ownerReferences to be same as the Kubernetes Deployment that was created for the odo component.

A mix of solutions 2 & 3 could be to set only one label app.kubernetes.io/managed-by: odo and set the ownerReferences. This is what me & @kadel discussed on one of the calls.

For the PVC error, I will have to dig further.

@kadel @feloy @mik-dass @valaparthvi thoughts?

@dharmit dharmit removed this from In Progress in Sprint 204 Jul 29, 2021
@dharmit dharmit moved this from To Do to In Progress in Sprint 205 Aug 2, 2021
@dharmit dharmit moved this from In Progress to For Review in Sprint 205 Aug 3, 2021
@kadel
Copy link
Member

kadel commented Aug 3, 2021

@kadel @feloy @mik-dass @valaparthvi thoughts?

I remember discussing this with you, @dharmit. Is there still something that you need?

If I remember it correctly the outcome was that we should make sure that the resources created from kubernetes component don't mix with the "main" resources that are part of the workspace (main component).
The ideal way to do it is to introduce extra label for the main pod and deployment, that odo will use to filter out resources that are part of the main component vs resources that were created from kubernetes component

@dharmit
Copy link
Member

dharmit commented Aug 3, 2021

I remember discussing this with you, @dharmit. Is there still something that you need?

Review the PR #4964. 😀

@dharmit
Copy link
Member

dharmit commented Aug 3, 2021

The ideal way to do it is to introduce extra label for the main pod and deployment, that odo will use to filter out resources that are part of the main component vs resources that were created from kubernetes component

The way I have gone about it in the code is to put only one label on Kubernetes inlined component - app.kubernetes.io/managed-by: odo.

@dharmit dharmit added this to For Consideration in Sprint 206 via automation Aug 16, 2021
@dharmit dharmit moved this from For Consideration to To Do in Sprint 206 Aug 16, 2021
@dharmit dharmit moved this from For Review to Done in Sprint 205 Aug 19, 2021
@dharmit dharmit moved this from To Do to In Progress in Sprint 206 Aug 26, 2021
@dharmit dharmit moved this from In Progress to For Review in Sprint 206 Sep 1, 2021
@dharmit dharmit added this to For Consideration in Sprint 207 via automation Sep 9, 2021
@dharmit dharmit moved this from For Consideration to To Do in Sprint 207 Sep 9, 2021
@dharmit dharmit moved this from To Do to For Review in Sprint 207 Oct 16, 2021
Sprint 206 automation moved this from For Review to Done Oct 21, 2021
Sprint 207 automation moved this from For Review to Done Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects
No open projects
Sprint 205
  
Done
Sprint 206
  
Done
Sprint 207
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants