Skip to content

Conversation

@cescgina
Copy link
Contributor

Create the WatcherAPI keystoneEndpoint. After this point, we can run
watcher commands from the openstackclient pod (even though we can't do
anything because the decision engine and applier are not available yet).

This version does not support TLS, so the control plane must be
deployed with TLS disabled as well.

Add tests for WatcherAPI keystoneendpoint addition, and remove specific ip from
watcher-api-service-override, as that can be environment-specific.

Related: OSPRH-11483

Depends-On: openstack-k8s-operators/ci-framework#2658

Comment on lines 483 to 489
if (ctrlResult != ctrl.Result{}) {
// We can ignore RequeueAfter as we are watching the KeystoneEndpoint
// resource
return ctrlResult, nil
}

return ctrl.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can remove the if block here and just return ctrlResult, nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, good catch, I'll push the change with the updated functional test

if (err != nil || result != ctrl.Result{}) {
// We can ignore RequeueAfter as we are watching the KeystoneEndpoint
// resource
return ctrl.Result{}, err
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return ctrl.Result{}, err
return result, err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, will push the fix in the next update

Create the WatcherAPI keystoneEndpoint. After this point, we can run
watcher commands from the openstackclient pod (even though we can't do
anything because the decision engine and applier are not available yet).

This version does not support TLS, so the control plane must be deployed
with TLS disabled as well.
@amoralej
Copy link
Contributor

This version does not support TLS, so the control plane must be deployed with TLS disabled as well.

Note that the fact that the operator does not support creating TLS endpoing does not imply that watcher can not run with a TLS enabled controlplane. Actually, I think we should be able to use a TLS enabled controlplane with #45 even if we are exposing watcher as http only.

Remove the loadBalancerIP from the watcher-api-service-override test.
This value will be environment specific, as environments might configure
different ip ranges in the IPAddressPools.
@cescgina cescgina force-pushed the create_watcherapi_keystone_endpoint branch from 72bd367 to 10f7e4b Compare January 21, 2025 11:59
@softwarefactory-project-zuul
Copy link

This change depends on a change that failed to merge.

Change openstack-k8s-operators/ci-framework#2658 is needed.

@cescgina
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link

This change depends on a change with an invalid configuration.

@cescgina
Copy link
Contributor Author

recheck

@amoralej
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented Jan 21, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amoralej

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@raukadah
Copy link
Contributor

/lgtm

Kuttl tests are validating it with via running commands in openstackclient pod.

 logger.go:42: 07:52:53 | watcher-api-service-override/1-deploy-with-defaults | + oc exec -n watcher-kuttl-default openstackclient -- openstack service list -f value -c Name -c Type
    logger.go:42: 07:52:53 | watcher-api-service-override/1-deploy-with-defaults | ++ grep -c '^watcher'
    logger.go:42: 07:52:55 | watcher-api-service-override/1-deploy-with-defaults | + '[' 1 == 1 ']'
    logger.go:42: 07:52:55 | watcher-api-service-override/1-deploy-with-defaults | ++ oc exec -n watcher-kuttl-default openstackclient -- openstack service list -f value -c Name -c Type -c ID
    logger.go:42: 07:52:55 | watcher-api-service-override/1-deploy-with-defaults | ++ awk '{print $1}'
    logger.go:42: 07:52:55 | watcher-api-service-override/1-deploy-with-defaults | ++ grep watcher

@openshift-ci openshift-ci bot added the lgtm label Jan 22, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit e1f3ef1 into openstack-k8s-operators:main Jan 22, 2025
6 checks passed
@cescgina cescgina deleted the create_watcherapi_keystone_endpoint branch January 22, 2025 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants