-
Notifications
You must be signed in to change notification settings - Fork 244
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
Adds support for linking using service binding without the service binding operator #4905
Adds support for linking using service binding without the service binding operator #4905
Conversation
311833f
to
a3a440c
Compare
@mik-dass You will need to skip the call to |
fbe69c7
to
d949218
Compare
pkg/service/service.go
Outdated
@@ -935,11 +943,17 @@ func (d *DynamicCRD) AddComponentLabelsToCRD(labels map[string]string) { | |||
|
|||
// PushServiceFromKubernetesInlineComponents updates service(s) from Kubernetes Inlined component in a devfile by creating new ones or removing old ones | |||
// returns true if the component needs to be restarted (when a service binding has been created or deleted) | |||
func PushServiceFromKubernetesInlineComponents(client *kclient.Client, k8sComponents []devfile.Component, labels map[string]string) (bool, error) { | |||
func PushServiceFromKubernetesInlineComponents(client *kclient.Client, k8sComponents []devfile.Component, labels map[string]string, ownerReferences metav1.OwnerReference, deployment *v1.Deployment) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this is suddenly accepting deployment
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use it to generate the owner reference and to regenerate the service binding object for deletion of it. If all the links are deleted, we can use this to get the deployment's name back.
There is a problem with the current approach. I have SBO and If I try to push devfile that has service and link devfile.yamlcommands:
- exec:
commandLine: mvn -Dmaven.repo.local=/home/user/.m2/repository spring-boot:run
component: tools
group:
isDefault: true
kind: run
hotReloadCapable: true
id: run
- exec:
commandLine: mvn clean -Dmaven.repo.local=/home/user/.m2/repository package -Dmaven.test.skip=true;
java -Xdebug -Xrunjdwp:server=y,transport=dt_socket,address=${DEBUG_PORT},suspend=n
-jar target/*.jar
component: tools
group:
isDefault: true
kind: debug
id: debug
components:
- container:
endpoints:
- name: 8080-tcp
targetPort: 8080
image: quay.io/eclipse/che-java11-maven:nightly
memoryLimit: 768Mi
mountSources: true
volumeMounts:
- name: m2
path: /home/user/.m2
name: tools
- name: m2
volume:
size: 3Gi
- kubernetes:
inlined: |
apiVersion: postgresql.dev4devs.com/v1alpha1
kind: Database
metadata:
name: database
spec:
databaseName: dbname
databasePassword: dbpass
databaseUser: dbuser
databaseStorageRequest: 1Gi
image: centos/postgresql-96-centos7
size: 1
name: database
- kubernetes:
inlined: |
apiVersion: binding.operators.coreos.com/v1alpha1
kind: ServiceBinding
metadata:
creationTimestamp: null
name: java-springboot-demo-qxdm-database-database
spec:
application:
group: apps
name: java-springboot-demo-qxdm-app
resource: deployments
version: v1
bindAsFiles: false
detectBindingResources: true
services:
- group: postgresql.dev4devs.com
kind: Database
name: database
version: v1alpha1
status:
secret: ""
name: java-springboot-demo-qxdm-database-database
metadata:
description: Spring Boot® using Java
displayName: Spring Boot®
globalMemoryLimit: 2674Mi
icon: https://www.eclipse.org/che/images/logo-eclipseche.svg
language: java
name: java-springboot
projectType: spring
tags:
- Java
- Spring
version: 1.1.0
schemaVersion: 2.0.0
starterProjects:
- git:
remotes:
origin: https://github.com/odo-devfiles/springboot-ex.git
name: springbootproject
I get
and sometimes
It looks like the problem is in the order in which resources are created. |
I have already pushed a fix for this issue. Please let me know it the new changes work for you. |
/test v4.7-integration-e2e |
@@ -32,7 +33,7 @@ var _ = Describe("odo link command tests for OperatorHub", func() { | |||
BeforeEach(func() { | |||
// wait till odo can see that all operators installed by setup script in the namespace | |||
odoArgs := []string{"catalog", "list", "services"} | |||
operators := []string{"redis-operator", "service-binding-operator"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be better to adapt the value of operators
depending on the value of os.Getenv("KUBERNETES")
7594e12
to
1d4fcd7
Compare
/retest |
1d4fcd7
to
2422e85
Compare
/retest |
7585728
to
c819588
Compare
/test v4.7-integration-e2e |
/test psi-kubernetes-integration-e2e |
It also separates the push related code for link and service creation.
pkg/component/component.go
Outdated
} | ||
|
||
for _, u := range list.Items { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can put this for
block in the if ok
block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
pkg/component/component.go
Outdated
@@ -1612,6 +1618,50 @@ func setLinksServiceNames(client *occlient.Client, linkedSecrets []SecretMount) | |||
} | |||
} | |||
|
|||
if !ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use else
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
/lgtm |
There is a problem.
|
The service and link related integration tests passed for me on a 4.7 cluster without the service binding operator. |
I've been testing this with following two devfiles, where the second one is slightly modified version what is in one of the tests and I'm not able to make it work with my crc. devfile.yaml
devfile.yaml
Always the same error
It looks like odo or something in odo is trying to get cluster wide Running |
The error comes from this line, the error seems to come from the Service Binding Operator library:
|
return false, err | ||
} | ||
|
||
secrets, err := client.ListSecrets(componentlabels.GetSelector(labels[componentlabels.ComponentLabel], labels[applabels.ApplicationLabel])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please catch this error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@kadel @feloy I have raised an issue on the service binding repo redhat-developer/service-binding-operator#1003 . I have also added a simpler error message for now in case the pipeline throws a forbidden type of error. |
…attempting creation of links
Kudos, SonarCloud Quality Gate passed! |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kadel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
hi i want to a qxdm tools ,do you have guys |
What type of PR is this?
/kind feature
What does this PR do / why we need it:
It adds support for linking using service binding without the service binding operator.
Which issue(s) this PR fixes:
Fixes #4940
Fixes #4543
PR acceptance criteria:
Unit test
Integration test
Documentation
Update changelog
I have read the test guidelines
How to test changes / Special notes to the reviewer:
Linking with other components
odo link <component-1> --context <component-0-directory>
odo push --context <component-0-directory>
Linking with services
odo link <service-name> --context <component-0-directory>
odo push --context <component-0-directory>
The option
--bind-as-files
should also work in the above scenarios.