-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: Adding first basic feast operator e2e test. #4791
base: master
Are you sure you want to change the base?
feat: Adding first basic feast operator e2e test. #4791
Conversation
Signed-off-by: lrangine <[email protected]>
Signed-off-by: lrangine <[email protected]>
Signed-off-by: lrangine <[email protected]>
Signed-off-by: lrangine <[email protected]>
_, err = utils.Run(cmd) | ||
ExpectWithOffset(1, err).NotTo(HaveOccurred()) | ||
|
||
var feastImage = "example.com/feature-transformation-server:operator.v0" | ||
By("loading the the feast image on Kind") | ||
var feastImage = "feastdev/feature-server:dev" |
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.
to be sure that k8s is looking locally and not trying to pull from a repo...
var feastImage = "feastdev/feature-server:dev" | |
var feastImage = "localhost/feastdev/feature-server:dev" |
registry: | ||
local: | ||
# This image is expected to build and to be available in the CI or when you run `make feast-ci-dev-docker-img` | ||
image: 'feastdev/feature-server:dev' |
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.
same
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.
Don't forget to add checks around the FeatureStore
CR status... checking ready state and individual feast service type conditions. Feel free to add whatever other CR status checks you think would be valuable. Generally, the more we test the better ... will help ensure a stable release.
// Get pod name | ||
|
||
cmd := exec.Command("kubectl", "get", | ||
"pods", "-l", "feast.dev/service-type=registry", |
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 wonder if the better route might be to check the registry deployment object and wait for availability condition to be true
. this will allow for easier testing around scaling later when we add that capability.
|
||
cmOutput, err := utils.Run(cmd) | ||
ExpectWithOffset(2, err).NotTo(HaveOccurred()) | ||
configMap := utils.GetNonEmptyLines(string(cmOutput)) |
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 would be good to validate the contents of the client configmap are as expected
services: | ||
registry: |
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 should test all 3 service types with this PR... onlineStore and offlineStore should be added
Adding first basic feast operator e2e test.
Issue we are fixing: 4792