Enable rbac and auth in e2e-smoke-test#516
Conversation
|
/assign @sebastienvas |
|
I tried to run istio/istio test on my RBAC cluster (which is RBAC for real now), and it failed. |
|
@kelseyhightower First, are you using my new test framework, if yes, did you add the flag --rbac_enable. Second, I admit when I tests locally, there is some flaky, I can't tell you what percentage, but well, not that bad. |
|
I will first create a post-submit for istio/istio to run extra tests, and then will add these to pilot/mixer postsubmit. |
|
@yutongz PR needs rebase |
sebastienvas
left a comment
There was a problem hiding this comment.
Sorry for the delay. Please take a look
| @@ -0,0 +1,45 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
All those scripts are basically the same. Can you make a main one and just call the main one from those with a flag ?
tests/e2e/framework/kubernetes.go
Outdated
| mixerTag = flag.String("mixer_tag", "", "Mixer tag, if different from istio.Version") | ||
| pilotHub = flag.String("pilot_hub", "", "pilot hub, if different from istio.Version") | ||
| pilotTag = flag.String("pilot_tag", "", "pilot tag, if different from istio.Version") | ||
| //caHub = flag.String("ca_hub", "", "Ca hub") |
There was a problem hiding this comment.
Can we uncomment those now ?
| //caHub = flag.String("ca_hub", "", "Ca hub") | ||
| //caTag = flag.String("ca_tag", "", "Ca tag") | ||
| authEnable = flag.Bool("auth_enable", false, "Enable auth") | ||
| rbacEnable = flag.Bool("rbac_enable", false, "Enable rbac") |
There was a problem hiding this comment.
Do you need those 2 flags ? can you assume rbacEnable when rbac file is specifed ?
There was a problem hiding this comment.
I was thinking about that, but from my point of view, using two bool flags make it easy to understand
tests/e2e/framework/kubernetes.go
Outdated
| return err | ||
| } | ||
|
|
||
| if *mixerHub != "" || *mixerTag != "" { |
There was a problem hiding this comment.
did you mean && ? you don t want any of those to be empty string I assume.
There was a problem hiding this comment.
I was trying to match the situation that only hub or tag is changed, but now I realize, you are right, we don't need to consider that situation. So change to &&
tests/e2e/framework/kubernetes.go
Outdated
| if *mixerHub != "" || *mixerTag != "" { | ||
| content = updateIstioYaml("mixer", *mixerHub, *mixerTag, content) | ||
| } | ||
| if *pilotHub != "" || *pilotTag != "" { |
|
@yutongz PR needs rebase |
|
Blocked by #550 |
|
Though still blocked by #550, I decided to split this pr into two steps since it starts to block other people. I made this pr backward compatible, let's try to get this in right after #545, and we can turn on two non-rbac e2e-suite first at istio/test-infra#362. I will deal with rbac part after #560 |
|
/test all |
1 similar comment
|
/test all |
|
Adding do-not-merge because the release note process has not been followed. |
|
/release-note-none |
|
/approve no-issue |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andraxylia, sebastienvas, yutongz The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/cancel do-not-merge |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue |
|
/test all |
* Update Jenkinsfile for qual * Fix bazelbuild rename issue * Fix postsubmit issues * Use sha instead of tag Former-commit-id: 11429422e29252adf597a0818c7c2dc2f29ce7bc
Automatic merge from submit-queue Enable rbac and auth in e2e-smoke-test Related issues: #484 #478 istio/test-infra#201 #186 Former-commit-id: da55d0a
* Update Jenkinsfile for qual * Fix bazelbuild rename issue * Fix postsubmit issues * Use sha instead of tag Former-commit-id: bd98dd6ec59f3a85bf809569b5e1d6be3d26624c
Automatic merge from submit-queue Enable rbac and auth in e2e-smoke-test Related issues: istio#484 istio#478 istio/test-infra#201 istio#186 Former-commit-id: da55d0a
Automatic merge from submit-queue Enable rbac and auth in e2e-smoke-test Related issues: #484 #478 istio/test-infra#201 #186 Former-commit-id: da55d0a
This now directly matches istio/istio. The problem here is it is impossible to override the default data source, and you can only have one default, so there are conflicts.
* [ior] MAISTRA-1400 Add IOR to Pilot * [MAISTRA-1089][MAISTRA-1400][MAISTRA-1744][MAISTRA-1811]: Add IOR to Pilot (istio#135) (istio#240) * MAISTRA-1400: Add IOR to Pilot (istio#135) * MAISTRA-1400: Add IOR to Pilot * [MAISTRA-1744] Add route annotation propagation (istio#158) * MAISTRA-1811 Store resourceVersion of reconciled Gateway resource (istio#190) * MAISTRA-1089 Add support for IOR routes in all namespaces (istio#193) * MAISTRA-2131: ior: honor Gateway's httpsRedirect (istio#276) If Gateway's httpsRedirect is set to true, create the OpenShift Route with Insecure Policy set to `Redirect`. Manual cherrypick from maistra/istio#269. * MAISTRA-2149: Make IOR robust in multiple replicas (istio#282) In scenarios where multiple replicas of istiod are running, only one IOR should be in charge of keeping routes in sync with Istio Gateways. We achieve this by making sure IOR only runs in the leader replica. Also, because leader election is not 100% acurate, meaning that for a small window of time there might be two instances being the leader - which could lead to duplicated routes being created if a new gateway is created in that time frame - we also change the way the Route name is created: Instead of having a generateName field, we now explicitly pass a name to the Route object to be created. Being deterministic, it allows the Route creation to fail when there's already a Route object with the same name (created by the other leader in that time frame). Use an exclusive leader ID for IOR * Manual cherrypick of maistra/istio#275 * MAISTRA-1813: Add unit tests for IOR (istio#286) * MAISTRA-2051 fixes for maistra install * MAISTRA-2164: Refactor IOR internals (istio#295) Instead of doing lots of API calls on every event - this does not scale well with lots of namespaces - keep the state in memory, by doing an initial synchronization on start up and updating it when receiving events. The initial synchronization is more complex, as we have to deal with asynchronous events (e.g., we have to wait for the Gateway store to be warmed up). Once it's initialized, handling events as they arrive becomes trivial. Tests that make sure we do not make more calls to the API server than the necessary were added, to avoid regressions. * MAISTRA-2205: Add an option to opt-out for automatic route creation If the Istio Gateway contains the annotation `maistra.io/manageRoute: false` then IOR ignores it and doesn't attempt to create or manage route(s) for this Gateway. Also, ignore Gateways with the annotation `istio: egressgateway` as these are not meant to have routes. * Add integration test for IOR Signed-off-by: Jacek Ewertowski <jewertow@redhat.com> * OSSM-1442: IOR: Ignore UPDATE events if resourceVersions are the same (istio#516) * OSSM-1442: IOR: Ignore UPDATE events if resourceVersions are the same For some obscure reason, it looks like we may receive UPDATE events with the new object being equal to the old one. As IOR always delete and recreate routes when receiving an UPDATE event, this might lead to some service downtime, given for a few moments the route will not exist. We guard against this behavior by comparing the `resourceVersion` field of the new object and the one stored in the Route object. * Add test Co-authored-by: Brian Avery <bavery@redhat.com> Co-authored-by: Jonh Wendell <jonh.wendell@redhat.com> Fix debug log formatting OSSM-1800: Copy gateway labels to routes Simplify the comparison of resource versions We store the gateway resource version (the whole metadata actually) in the `syncRoute` object. There's no need to loop over the routes to perform the comparison. This also fix the corner case where the gateway has one host and for some reason OCP rejects the creation of the route (e.g., when hostname is already taken). In this case the `syncRoute` object exists with zero routes in it. Thus the loop is a no-op and the function wrongly returns with an error of `eventDuplicatedMessage`. By comparing directly using the `syncRoute.metadata` we fix this. OSSM-1105: Support namespace portion in gateway hostnames They are not used by routes, so we essentially ignore the namespace part - anything on the left side of a "namespace/hostname" string. OSSM-1650 Make sure initialSync and event loop behave the same (istio#551)
…istio#516) * OSSM-1442: IOR: Ignore UPDATE events if resourceVersions are the same For some obscure reason, it looks like we may receive UPDATE events with the new object being equal to the old one. As IOR always delete and recreate routes when receiving an UPDATE event, this might lead to some service downtime, given for a few moments the route will not exist. We guard against this behavior by comparing the `resourceVersion` field of the new object and the one stored in the Route object. * Add test
…istio#516) * OSSM-1442: IOR: Ignore UPDATE events if resourceVersions are the same For some obscure reason, it looks like we may receive UPDATE events with the new object being equal to the old one. As IOR always delete and recreate routes when receiving an UPDATE event, this might lead to some service downtime, given for a few moments the route will not exist. We guard against this behavior by comparing the `resourceVersion` field of the new object and the one stored in the Route object. * Add test
Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
commit 466ae697aa72eac4172ab2f88d4d66c0d90d053d
Author: Yang Liu <yannliu@redhat.com>
Date: Thu Mar 23 04:22:40 2023 +0800
OSSM-1689 Simplify IOR (istio#747)
* Rework IOR initialization
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove `initialSync`
`initialSync` is not needed.
- During boostrap, `SetNamesapces`is always called with no namespaces.
- When removing or adding a namespace, the underlaying informer will
trigger an `ADD` event for all resources the informer watches
Signed-off-by: Yann Liu <yannliu@redhat.com>
* DIsable TestPref
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Rename
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Call `findService` once for each gateway
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Use original host to generate Route name
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Skip duplicate update test
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Improve concurrency test
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Introduce update Route on Gateway update
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix data race
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Format and lint
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Respect log level
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Refactor IOR
- `gatawayMap` is removed. `Routes` are retrived via API.
- `reconcileGateway` is used to achieve the desired state.
- `processEvent` will only process the latest and try to abort early.
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove unused functions
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Use `Lister` for finding target service
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Start IOR before kube client
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove unused properties
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Rework test initialization
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Log correct debug information
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove unnecessary parameters
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove ResourceVersion usage
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Avoid deletion of a route when failing to update
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Update FakeRouter to record API call counts
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Rework initialization
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Keep startup process order consistent
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix creating matching service
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Test IOR to be idempotent
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove unused parameters
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Rename symbol
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Remove used struct
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Improve styling and wording
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Add support list across namespaces in faker
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Lint and format
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Introduce Openshift Route informer
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Lint
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Run make gen
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix data race
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix test data race
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Lint
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Rename variables
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix update route
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Linit
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Increase wait for the delete
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Maximize time to wait for the route deletion
* Fix route update
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix route update
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Test with a 30 second wait
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Fix flaky test
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Add disabling IOR and clean up
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Defer clean up
Signed-off-by: Yann Liu <yannliu@redhat.com>
* Clear only ior routes
Signed-off-by: Yann Liu <yannliu@redhat.com>
* rename newRoute to newRouteController
* rename route.go to controller.go
---------
Signed-off-by: Yann Liu <yannliu@redhat.com>
Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Signed-off-by: Yann Liu <yannliu@redhat.com>
commit afe4692e69c68f7dc504550dd4770e4057431508
Author: Jonh Wendell <jonh.wendell@redhat.com>
Date: Wed Nov 16 08:10:44 2022 -0500
OSSM-2256: Add IOR (istio#680)
* [ior] OSSM-2256: Add IOR
* [ior] MAISTRA-1400 Add IOR to Pilot
* [MAISTRA-1089][MAISTRA-1400][MAISTRA-1744][MAISTRA-1811]: Add IOR to Pilot (istio#135) (istio#240)
* MAISTRA-1400: Add IOR to Pilot (istio#135)
* MAISTRA-1400: Add IOR to Pilot
* [MAISTRA-1744] Add route annotation propagation (istio#158)
* MAISTRA-1811 Store resourceVersion of reconciled Gateway resource (istio#190)
* MAISTRA-1089 Add support for IOR routes in all namespaces (istio#193)
* MAISTRA-2131: ior: honor Gateway's httpsRedirect (istio#276)
If Gateway's httpsRedirect is set to true, create the OpenShift Route
with Insecure Policy set to `Redirect`.
Manual cherrypick from maistra/istio#269.
* MAISTRA-2149: Make IOR robust in multiple replicas (istio#282)
In scenarios where multiple replicas of istiod are running,
only one IOR should be in charge of keeping routes in sync
with Istio Gateways. We achieve this by making sure IOR only
runs in the leader replica.
Also, because leader election is not 100% acurate, meaning
that for a small window of time there might be two instances
being the leader - which could lead to duplicated routes
being created if a new gateway is created in that time frame -
we also change the way the Route name is created: Instead of
having a generateName field, we now explicitly pass a name to
the Route object to be created. Being deterministic, it allows
the Route creation to fail when there's already a Route object
with the same name (created by the other leader in that time frame).
Use an exclusive leader ID for IOR
* Manual cherrypick of maistra/istio#275
* MAISTRA-1813: Add unit tests for IOR (istio#286)
* MAISTRA-2051 fixes for maistra install
* MAISTRA-2164: Refactor IOR internals (istio#295)
Instead of doing lots of API calls on every event - this
does not scale well with lots of namespaces - keep the state
in memory, by doing an initial synchronization on start up and
updating it when receiving events.
The initial synchronization is more complex, as we have to deal with
asynchronous events (e.g., we have to wait for the Gateway store to
be warmed up). Once it's initialized, handling events as they arrive
becomes trivial.
Tests that make sure we do not make more calls to the API server than
the necessary were added, to avoid regressions.
* MAISTRA-2205: Add an option to opt-out for automatic route creation
If the Istio Gateway contains the annotation `maistra.io/manageRoute: false`
then IOR ignores it and doesn't attempt to create or manage route(s) for
this Gateway.
Also, ignore Gateways with the annotation `istio: egressgateway` as
these are not meant to have routes.
* Add integration test for IOR
Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
* OSSM-1442: IOR: Ignore UPDATE events if resourceVersions are the same (istio#516)
* OSSM-1442: IOR: Ignore UPDATE events if resourceVersions are the same
For some obscure reason, it looks like we may receive UPDATE events with
the new object being equal to the old one. As IOR always delete and
recreate routes when receiving an UPDATE event, this might lead to some
service downtime, given for a few moments the route will not exist.
We guard against this behavior by comparing the `resourceVersion` field
of the new object and the one stored in the Route object.
* Add test
Co-authored-by: Brian Avery <bavery@redhat.com>
Co-authored-by: Jonh Wendell <jonh.wendell@redhat.com>
Fix debug log formatting
OSSM-1800: Copy gateway labels to routes
Simplify the comparison of resource versions
We store the gateway resource version (the whole metadata actually) in the `syncRoute` object.
There's no need to loop over the routes to perform the comparison.
This also fix the corner case where the gateway has one host and for
some reason OCP rejects the creation of the route (e.g., when hostname is already
taken). In this case the `syncRoute` object exists with zero routes in
it. Thus the loop is a no-op and the function wrongly returns with an
error of `eventDuplicatedMessage`. By comparing directly using the
`syncRoute.metadata` we fix this.
OSSM-1105: Support namespace portion in gateway hostnames
They are not used by routes, so we essentially ignore the namespace part
- anything on the left side of a "namespace/hostname" string.
OSSM-1650 Make sure initialSync and event loop behave the same (istio#551)
* OSSM-1301 Wait for Route resource type to become available on ior startup (istio#631)
* OSSM-2109 Fix flaky IOR unit test (istio#648)
The sleep in ensureNamespaceExists was hardcoded to 100ms, regardless of r.handleEventTimeout. This timeout during unit tests is only 1ms, so the 100ms sleep caused the for loop to only run once.
Here we change the duration of the sleep to be 1/100 of r.handleEventTimeout. This change preserves the production sleep time of 100ms, but reduces the sleep time in unit tests to 10μs. This makes ensureNamespaceExists() run the for loop multiple times before giving up, fixing the test's flakiness.
Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
* OSSM-2006 Fix multiNamespaceInformer.HasSynced()
Co-authored-by: Jacek Ewertowski <jewertow@redhat.com>
Co-authored-by: Marko Lukša <marko.luksa@gmail.com>
Co-authored-by: maistra-bot <57098434+maistra-bot@users.noreply.github.com>
Signed-off-by: Yann Liu <yannliu@redhat.com>
Signed-off-by: Yann Liu <yannliu@redhat.com>
Related issues:
#484
#478
istio/test-infra#201
#186