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 portName in HTTPScaledObject service scaleTargetRef #1174

Merged
merged 7 commits into from
Oct 25, 2024

Conversation

wozniakjan
Copy link
Member

The HTTPScaledObject allowed setting only numerical port value for the scaled services. For improved UX, it also allows to set namedPort to match common conventions in the kubernetes API regarding Services and ports.

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO)
  • Changelog has been updated and is aligned with our changelog requirements

@wozniakjan wozniakjan requested a review from a team as a code owner October 23, 2024 13:30
@wozniakjan wozniakjan force-pushed the allow_portName_in_httpso_service branch 2 times, most recently from c986503 to bcd5e45 Compare October 23, 2024 13:42
@wozniakjan wozniakjan force-pushed the allow_portName_in_httpso_service branch from bcd5e45 to 9d1e194 Compare October 23, 2024 13:45
Copy link
Contributor

@jkremser jkremser left a comment

Choose a reason for hiding this comment

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

👍 maybe they should be mutually exclusive, can be done using x-kubernetes-validations. If not, we can check that at least one of them is set, also by these X rules. Something like:

// +kubebuilder:validation:XValidation:rule=(has(self.PortName) || has(self.Port))

operator/apis/http/v1alpha1/httpscaledobject_types.go Outdated Show resolved Hide resolved
operator/apis/http/v1alpha1/httpscaledobject_types.go Outdated Show resolved Hide resolved
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Looking good, e2e test would be nice

@wozniakjan
Copy link
Member Author

wozniakjan commented Oct 23, 2024

maybe they should be mutually exclusive, can be done using x-kubernetes-validations.

This is neat! I will explore it a bit more and definitely find a place for this in my toolset :)

Looking good, e2e test would be nice

right, I knew I forgot something, will followup shortly :)

wozniakjan and others added 3 commits October 24, 2024 09:27
Co-authored-by: Jirka Kremser <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
Signed-off-by: Jan Wozniak <[email protected]>
@wozniakjan wozniakjan force-pushed the allow_portName_in_httpso_service branch from bb59ad6 to acfe4bd Compare October 24, 2024 09:18
Signed-off-by: Jan Wozniak <[email protected]>
@wozniakjan wozniakjan force-pushed the allow_portName_in_httpso_service branch 3 times, most recently from 17de409 to 8b66f00 Compare October 24, 2024 13:14
@wozniakjan wozniakjan force-pushed the allow_portName_in_httpso_service branch from 8b66f00 to a98dab2 Compare October 24, 2024 13:16
Comment on lines +15 to +22
- apiGroups:
- ""
resources:
- services
verbs:
- get
- list
- watch
Copy link
Member Author

Choose a reason for hiding this comment

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

highlighting that newly services RBAC will be necessary for the interceptor

@wozniakjan wozniakjan merged commit f5ab058 into kedacore:main Oct 25, 2024
18 checks passed
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.

3 participants