use current time for running daily release by rkpagadala · Pull Request #3507 · istio/istio

rkpagadala · 2018-02-15T01:51:13Z

No description provided.

ldemailly · 2018-02-15T06:31:39Z

release/pipline/dags/istio_daily_flow.py

that was some weird UTC / date mix is it not anymore? 8a pst is probably too late if it takes 1h+ to make let's keep it around 1am or maybe up to 4a but not later ?

this was on request of @hklai

We are trying to align the release cut off time and pipeline schedule.

The idea is to have a new daily build in the morning PST. We can do 6:15am so we normally can get a daily release by 07:30-ish PST.

@hklai and @rkpagadala when back and forth on this question. There will always be people submitting code at midnight who want it in the build and others that want the build available first thing in the morning so there is no right answers. If we had people in New York working on istio 8 AM would be 11 AM witch is quite late. However under the current green build system(runs every two hours) you need to get code in by 11 PM for it to be in the next days daily. Best that we atlest make sure all code from day -1 is part of the build. The key is to not keep changing it or everyone will be confused.

right now it seems it changed to noon somehow which is close to the worst possible time

let's do the builds between 1a and 4a PST

I think the execution_time problem made it a lot more confusing, when the build cut at 1am was labeled as one day before.

With that now fixed, I am fine with sticking with 1am. However, that practical means the release candidate cut off day is on 14th 1am PST.

I will amend the release process to make it clear.

if you prefer to make it 11:59pm (23:59) so it stays 'end' of the 14th or 01:00a on the 15 it's all good/fine - up to you as long as it's clear and at night (and I don't mind 1a 14th either)

thanks!

fun fact: we have to be careful to not pick 2a local time for instance that can get a) skipped, b) ran twice during daylight savings

rkpagadala · 2018-02-15T20:42:32Z

/retest

hklai · 2018-02-15T18:58:44Z

release/pipline/dags/istio_common_dag.py

Let's add a comment to explain the problem of execution_date.

hklai · 2018-02-16T01:07:14Z

release/pipline/dags/istio_daily_flow.py

We are trying to align the release cut off time and pipeline schedule.

The idea is to have a new daily build in the morning PST. We can do 6:15am so we normally can get a daily release by 07:30-ish PST.

hklai · 2018-02-16T16:53:38Z

release/pipline/dags/istio_daily_flow.py

I think the execution_time problem made it a lot more confusing, when the build cut at 1am was labeled as one day before.

With that now fixed, I am fine with sticking with 1am. However, that practical means the release candidate cut off day is on 14th 1am PST.

I will amend the release process to make it clear.

wattli · 2018-02-17T01:42:23Z

prow/OWNERS

  - sebastienvas
  - yutongz
-
+  - rkpagadala


please sort alphabetically, same below.

run daily at 1 am pst, tag daily releases

ldemailly · 2018-02-19T21:47:08Z

/lgtm
btw please see #3609

istio-merge-robot · 2018-02-19T21:47:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ldemailly

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these OWNERS Files:

~~prow/OWNERS~~ [ldemailly]
~~release/OWNERS~~ [ldemailly]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

istio-merge-robot · 2018-02-19T22:14:36Z

/test all [submit-queue is verifying that this PR is safe to merge]

istio-merge-robot · 2018-02-19T22:41:15Z

Automatic merge from submit-queue.

istio-testing · 2018-02-19T23:11:01Z

@rkpagadala: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/istio-pilot-e2e.sh	`b7f21e3`	link	`/test istio-pilot-e2e`

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

hklai

/lgtm

hklai · 2018-02-20T01:08:14Z

release/pipline/dags/istio_common_dag.py

    config_settings['GCS_FULL_STAGING_PATH'] = '{}/{}'.format(
        config_settings['GCS_STAGING_BUCKET'], gcs_path)
    config_settings['ISTIO_REPO'] = 'https://github.com/{}/{}.git'.format(
-        config_settings['GITHUB_ORG'],  config_settings['GITHUB_REPO'])


extra space intentional?

we removed extra space, what is remaining is intentional

hklai · 2018-02-20T01:08:59Z

release/pipline/dags/istio_common_dag.py

+      bash_command=gcr_tag_success,
+      dag=dag,
+  )
+  # skip_grc = DummyOperator(


will followup in a different PR

ldemailly · 2018-02-20T02:07:28Z

sorry it merged already

* WorkloadInstance gets correct clusterID * Workloads have a draining weight The amount of traffic is controlled by the draining weight which is a value from 0 to 100. The default is 0 which means no draining. When the weight is 100, no traffic will be sent to that endpoint.

rkpagadala requested a review from a team February 15, 2018 01:51

googlebot added the cla: yes label Feb 15, 2018

ldemailly reviewed Feb 15, 2018

View reviewed changes

rkpagadala requested review from a team, hklai and sebastienvas February 15, 2018 18:23

hklai reviewed Feb 16, 2018

View reviewed changes

rkpagadala added 2 commits February 16, 2018 18:20

use current time for running daily release

05adaea

change owners for prow and release

a00f58a

wattli reviewed Feb 17, 2018

View reviewed changes

prow/OWNERS Outdated

- sebastienvas

- yutongz

- rkpagadala

Copy link
Copy Markdown

Contributor

wattli Feb 17, 2018

Choose a reason for hiding this comment

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

please sort alphabetically, same below.

rkpagadala added 2 commits February 17, 2018 03:08

sort owners

6407f52

use version for monthly release

b7f21e3

run daily at 1 am pst, tag daily releases

istio-testing assigned ldemailly Feb 19, 2018

istio-testing added the lgtm label Feb 19, 2018

istio-merge-robot added the approved label Feb 19, 2018

istio-merge-robot merged commit 7acfd1e into istio:master Feb 19, 2018

hklai reviewed Feb 20, 2018

View reviewed changes

Conversation

rkpagadala commented Feb 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldemailly Feb 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkpagadala commented Feb 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldemailly commented Feb 19, 2018

Uh oh!

istio-merge-robot commented Feb 19, 2018

Uh oh!

istio-merge-robot commented Feb 19, 2018

Uh oh!

istio-merge-robot commented Feb 19, 2018

Uh oh!

istio-testing commented Feb 19, 2018

Uh oh!

hklai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldemailly commented Feb 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ldemailly Feb 17, 2018 •

edited

Loading