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
update apiservice registration namespace and spec on initialization #935
Conversation
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.
In general great, we need that.
On second thought, we may need a check if there is another virt-api server running in the other namespace too and simply fail to start then. I think that otherwise the virt-api servers in different namespaces would just register themselves back and forth ...
@davidvossel what do you think?
@rmohr I updated this patch to fail early if a namespace mismatch occurs. This puts the resolution on the operator. The error message clearly states what the issue is and how to resolve it. |
pkg/virt-api/api.go
Outdated
apiService.Spec.CABundle = app.signingCertBytes | ||
} else { | ||
if apiService.ObjectMeta.Namespace != namespace { | ||
return fmt.Errorf("apiservuce [%s] is already registered in a different namespace. Existing apiservice registration must be deleted before virt-api can proceed.", subresourceAggregatedApiName) |
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.
s/apiservuce/apiservice
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.
oops, fixed :)
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.
Looks perfect. Can go in as soon as the type is fixed and ci looks fine.
retest this please |
@cynepco3hahue I think that something goes wrong during the aggregated apiserver registration. |
@rmohr Looks so, I see
|
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.
Ok, it fails in a virt-api scale-out environment.
pkg/virt-api/api.go
Outdated
// Update apiService if CA bundle doesn't match ours | ||
apiService.Spec.CABundle = app.signingCertBytes | ||
} else { | ||
if apiService.ObjectMeta.Namespace != namespace { |
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.
Ha, now I see the issue. That should be:
if apiService.Spec.Service.Namespace != namespace {
}
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.
whoa, hooray for functional tests :)
…spaces exist Signed-off-by: David Vossel <davidvossel@gmail.com>
Signed-off-by: Alexander Wels <awels@redhat.com>
related to #932