Initial Blue Green Strategy #1
Conversation
dthomson25
force-pushed
the
PR-branch
branch
from
ddc7368 to
e595267
Compare
| @@ -56,22 +40,23 @@ func main() { | |||
| klog.Fatalf("Error building kubernetes clientset: %s", err.Error()) | |||
There was a problem hiding this comment.
Unless I'm missing something klog by default sends logs to files, not stdout. It can be configured using --logtostderr flag but we need to run klog.InitFlags(nil).
There was a problem hiding this comment.
Addressed when we switched away from klog: #7
| @@ -217,7 +221,7 @@ func (c *Controller) processNextWorkItem() bool { | |||
| return nil | |||
| } | |||
| // Run the syncHandler, passing it the namespace/name string of the | |||
| // Foo resource to be synced. | |||
| // Rollout resource to be synced. | |||
| if err := c.syncHandler(key); err != nil { | |||
| // Put the item back on the workqueue to handle any transient errors. | |||
| c.workqueue.AddRateLimited(key) | |||
There was a problem hiding this comment.
I think we need to reschedule only if error is transient. I've created invalid rollout (with invalid active service name ) and controller kept trying to synchronize it forever. I don't know how to distinguish which error is transient so I would suggest to remove this rescheduling.
| // The number of old ReplicaSets to retain. | ||
| // This is a pointer to distinguish between explicit zero and not specified. | ||
| // +optional | ||
| RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty"` |
| // 2) Max number of pods allowed is reached: deployment's replicas + maxSurge == all RSs' replicas | ||
| func NewRSNewReplicas(rollout *v1alpha1.Rollout, allRSs []*appsv1.ReplicaSet, newRS *appsv1.ReplicaSet) (int32, error) { | ||
| switch rollout.Spec.Strategy.Type { | ||
| case v1alpha1.BlueGreenRolloutStrategyType: |
There was a problem hiding this comment.
Strategy field is marked as optional. I think controller should use BlueGreen rollout strategy is Strategy field is not specified (e.g. rollout.Spec.Strategy.Type == "")
There was a problem hiding this comment.
My concern with blue green is that it requires an active service, and it won't be set if the Strategy field is empty. What do you think about using a rolling-update once it's implemented as a default and have an invalid spec condition that says that the strategy needs to be filled out in the meanwhile.
|
|
||
| // Wait for the caches to be synced before starting workers | ||
| klog.Info("Waiting for informer caches to sync") | ||
| if ok := cache.WaitForCacheSync(stopCh, c.deploymentsSynced, c.foosSynced); !ok { | ||
| if ok := cache.WaitForCacheSync(stopCh, c.replicaSetSynced, c.rolloutsSynced); !ok { |
There was a problem hiding this comment.
I've tried to create rollout without strategy and controller never starts. WaitForCacheSync stuck forever
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: example-rollout
spec:
replicas: 1
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:1.15.4
ports:
- containerPort: 80
minReadySeconds: 30
revisionHistoryLimit: 3
|
After discusing with the team, I'm merging in this pr and I will address this feedback in future PRs |
…sses feat: add support for N nginx ingresses per service
integrate multi-alb
No description provided.