GitLab Release Group, 30 Jul 2021

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Sync on Invalid Environments

Description

Daniel Fosco (Senior Product Designer, Release) and Shinya Maeda (Senior Backend Engineer, Release) discuss the issue "Invalid Environment Not Being Created Should Be Surfaced To User"

https://gitlab.com/gitlab-org/gitlab/-/issues/21182

A

Hello: everyone I'm here with chenia maeda, a senior rebecca developer on the release group. uh We are discussing today an issue. Let me share my screen as well, so it's clear on the recording.

A

Before invalid environment, not being created uh should be surface to the user, so, as the title says, essentially, this issue is about pipelines that create an environment where there's an error in the environment creation, so it's not created, even though the pipeline may succeed, and that error is not surface to the user right now, so we are discussing ways to service this to the user in a way that makes sense. um Does that sound correction? Yeah? Yes,.

B

Yeah all right, so uh so, um if I add one point, this issue is talking about, environment is not created, but technically there are two buttons that environment is not created and the environment is not updated.

B

uh Yeah I can demonstrate uh you know that today's user's experience, if you want like creating a dummy project and what users actually see today,.

A

B

Then let me share my screen so.

B

Sorry, I don't have any preparation, so I'm not sure if this demo goes well but let's just uh create a new project.

B

Add an environment.

B

B

B

So uh here is a demo project and let's set up the csd pipeline.

B

uh In this demo, we gonna wrap you up.

B

It's a deployment, script done script and then environment.

B

This is the part that uh coming from branch knee so.

B

Let's commit this file, so we just set up this.

A

Sorry, this variable it maps to the branch name or to the commit name. Yeah.

B

Yeah yeah uh branch name right now, you will see so uh this is our cd pipeline we just created in for this project so for initiating the review app. Let's create a modular cast, I'm going to edit this readme, whatever thing and then future branch.

B

I don't change with me and create a mod request.

B

There you go so I created this much request and then pipeline runs. um So, as you can see in that this review branch name, this environment is created successfully. So this is a expected behavior. Everything went well, but.

B

For example, if we create moderate with a branch name that includes special keyword, that is allowed for git ref, but it's not allowed for environment name.

B

It's going to be.

B

So user creates a successfully march request and then they see this pipeline, like everything, seems normal right, but they don't say impairment here.

B

B

You know that if things goes well, uh users should see environment, slash future uh dash exclamation mark, but the ex crowmation market is not allowed for environment records, so the expected environment is not created, so this is uh exactly the case that environments is not created um with the pipeline.

B

um So the first question is that how do we present this error? Surface dcl for users.

A

Right, even before that, uh you probably have the answer to that question. But um why are the the character records for environments? Not the same set as for the forget, like is? Is that a change that we could make.

B

B

I I don't know the reason um just uh I think it makes sense. The like your point is rory you're, making very good point that maybe we should uh make that the ability to set that use the same validation between give the breath and the environment name um but yeah. It is what it is today.

A

Yeah, I I mean there may be some big technical reason for it.

B

A

Adding an error alert is: is the smallest iteration that we could make to to improve this, but yeah, maybe something that we can think in the future, because then, like we just cut down the the root of the error in the error. Just at least this kind of error doesn't happen anymore right. The.

B

Environment they.

A

Are for other reasons, and then it's good to have the alert, but but this is no longer one of them. Perhaps.

B

Yeah, uh I just remember that why we don't a lot of special keywords in environments, that environment has a feature to generalize, slug, so url for environment. um So this url doesn't allow special keywords, uh all right, so yeah, that's a reason.

B

um If you don't mind, I can continue to the environment, update failure. So do you want to spend time on this.

A

Yeah we can go on.

B

Okay, so the next case, the environment, update failure is a bit tricky.

B

But actually happening on user's project. Sorry, so.

B

B

In the cd pipeline, users can also set the url uh something like https. uh What's what's a good example for this, oh yeah.

B

So, um for example, like a setting- you are like this, but you could be, could be in buried. Sometimes, for example,.

B

I don't know just invalid something: this is a invalid url.

B

Update the cd pipeline, let's create a modular guest.

B

B

One this is a valid environment name, so we should see.

B

The environment is created so.

B

This game is going well right, but I think that this the system fails to update this environment after the deployment job has finished.

B

uh Maybe we can check some.

B

This is the environment we just created.

B

Success, how can we check that frequency.

B

ah Wait wait. uh I get a better example.

B

How can I do that.

B

Let's go to my test so.

B

Let's, uh let's back to the first module, I guess that created environment successfully.

B

We change this pipeline definition to let's say the url valid url.

B

Okay, they come in.

B

Okay, now, uh the second pipeline is.

B

Running right, um then,.

B

Job finished, then we see the environment here.

B

Okay, so um this is a uh first module. uh The review app that's created by the first smart request, and then we just set the url right, the google website and then, since ul, is correctly set to this environment. Record users see this view deployment button to go to this linked website.

B

So this is expected behavior, but.

B

Back to the uh the last smart request, we created.

B

In this module guest.

B

We set the wrong reset invalid url right like this one. So if it's invalid.

B

The url error cannot be set to you uh environment. Therefore, you said: don't you either? Don't see this button, the jump to the deployment environment page button, so um the system tried to update the environment record after the deployment finished, um but that since url is invalid, uh this system failed in this case implicitly failed. um So you know users will be confused that hey. I just set the url for the this environment, but I don't see anything. I don't see even errors what's going on here.

B

um This is a very explicit uh example, but the uh in real case it's much more hard to uh determine their diagnosis problem, because it looks like, for example,.

B

Like this, is this valid ul or whatnot? um It's. I think this is invalid, because uh gitlab internally doesn't allow http protocol only https, um so.

B

Like uh in this case, users will be really confused, like at the class. The url looks valid, but uh something failed uh in the back end yeah. That's how the the second case that environment update failure um and then back to the issue that uh maybe a you can take it over, though how we, you know, service these llrs to users that so that they can take an action, but at least they have an idea like something went wrong, and then they yeah.

B

Ideally the exact error message that lets them: users to take an action to fix their city pipelines.

A

Yeah, I think the first thing that comes to mind for me is is going over the current behavior of the the pipelines when these things happen. So right now, on all of these cases, where there was an error to the department being created, the pipeline still succeeded right. Yes, yes, failure to create an environment is not considered a failure by by our rules on on running the jobs. So the first thing I would ask is that that is uh that doesn't seem to be the ideal state of things.

A

So do we consider that the pipeline jobs should output a different state in this case um and if so, which state? Because just saying success is not entirely true right.

B

Is it tricky that um when job succeeded, it mostly means that job script succeeded so in this case job scripts? um We just implemented the dummy diplomatic code, though the scripts really succeeded, so um the simplest idea would be for the create a create problem. The uh the first problem- it's just failed.

B

Maybe it makes sense to just fail the deployment, the job sorry for smart requesting, which one uh wait, wait yeah this one, the second module is we created uh that includes this invalid character. Maybe these deployment jobs should fail, and then we show a message that uh environment, failed environment creation failed because this character.

B

Yes, that makes sense because.

A

For for me, it makes sense that traditionally, uh jobs are were used for testing and ai purposes, so it makes sense that the success of the job is a reflection of the success of the script that the job is running. But if you're talking about the deployment script, um if the deployment script succeeds, but the deployment action does not, then then, then the deployment job failed right.

B

No idea, I agree.

A

It's a matter for us to deciding if we want to fully fail that job or if we want to introduce some distinction. I think it was you that mentioned uh past with errors that we currently don't have. We just have passed with warnings right. um I think the the way these are set up uh if we abstract away from from the specific errors, it's saying like the script succeeded, but the infrastructure around it failed, failed right.

B

Something external.

A

To the script has failed, so maybe it is the case of either failing everything like if a part external to the script fails uh on a deployment, job or any job, then we fail the whole job or we implement some sort of state that is passed with errors to tell the user hey it passed with errors, and what that means is the actual script of the job is successful, like you, don't have to fix your code or fix the testing script, but but external factors failed, and these are the factors.

B

uh I agree with that. um The maybe the first proposal for this issue that we should fail. The deployment job.

A

I think that's simpler for that.

B

Yes, yes, yes, the only concern in this path is that it's going to be. It could be the disturbing change for users, some users, for example. We are recording currently tracking the this case um in this era tracking system, and we can see that, uh like 300 600, 000 events uh every day, uh users are wrongly using this um after we started failing these jobs.

B

These users are gonna face, um like maybe they're, be surprised that why my deep limiters suddenly started, failing, which is correct behavior, but uh you know, um the tricky problem is how we gracefully roll up this change. We definitely use a future flag, at least uh maybe communicate with customers in advance. um um Yeah. uh Probably that actually is actions will be needed.

B

Well, I think we can almost ugly with the first the proposal for the first case, um though kind of the you know, I don't really have a good idea how we address the second case, the which is up environment, update failure, uh so in this demo project it's a third launch request.

B

So do you think this job should also fail.

A

So uh remind me, which case was this like which error.

B

uh The second case that environment uh update, failed environment creation, succeeded, but environment update failed because in this case, environment url is invalid. Yes,.

A

Right um yeah, I think I mean if it failed to update, then then it should be a failure still.

B

The failure still yeah.

A

Because if you think about it, sure the the the the deployment the deployable artifact was deployed right, the environment was updated, but then we broke the url, so this might break other things in the system, like sure maybe for their system. Http is fine, so nothing breaks in there and it's broken from the gitlab perspective, but we don't know that right and the error might be first, the first uh variation we did like with a completely environmental url, um so we currently have no way of knowing that so out of safety.

A

I would fail everything, um but I think this is also a product decision with how we want to approach it, and if we want to go this route just figuring out, how do we communicate with our user base to make sure that it's almost like a deprecation in a way like we have to? Let them know in advance that something that was currently working, at least from their perspective, will no longer be working right. We will start breaking something that we were communicating was not breaking, uh so their errors will be blown up so.

B

Yeah yeah yeah, the the tricky point is that the error is hard, whether heart failure or soft software or hot era. um I don't know if all inbound cases is really critical for users um like we're starting making it harder failure, though um yeah uh so, um which.

A

Question yeah: you you mentioned having a feature flag for this and gave me the idea. Should we roll out this option as a as a setting that is obtained by default?

A

A

Should we we introduce this change as a setting that is opting by default, so we roll out to every users and then some users say: oh, my errors are blowing up and then they see our blog posts or communications saying oh, this is actually a setting that you can turn off. So if you have.

B

An organization.

A

Like I don't care, if environments fail and our in our use.

B

A

Make a difference: you can turn it off and then you won't register as errors.

B

What are you just referring? It's project configuration project level configuration not future flag. Future flag is um the the kind of similarity project level configuration, but only gitlab can yeah control, so um it's uh yeah we can. We can also go down this path, like maybe the creator setting csd and then like fail uh fail. I don't know check but could get another checkbox that failed deployment job. If and if the system failed environment creation will update. uh That's also uh yeah one of the approaches.

A

Yeah um so um just to start wrapping it up because we're almost the time, I think the options that we have are either to make make environment creation, failure and environment update, update failure to fail the jobs entirely like it goes red, but then we have to figure out how to communicate that to our users.

A

Another option is to introduce a new state that is passed with errors, so we communicate that and then, in both cases we would have an alert component on the job log page right.

B

So two comments: introducing a new status is definitely the listing. The last result we did. We do not want to introduce yet another status. uh It's a very complicated big change, right yeah, and the second thing is that, should we split this issue into two uh like one is create one and then the other one is update failure. um It seems, like uh you know, for the cleared case, create a failure case. We almost have a. We were also certain that the failing job would be the way to go.

B

um So uh you know we're just addressing this specific problem for something uh utility on uh for the up environment, update failure in the next iteration.

A

um I mean if, if you think it's it's valid, we can do it. I don't know from the back in perspective if it's, if it's worth separating, but if you think so it makes sense. Yeah, I think, from the from the experience perspective, the experience should be the same.

B

uh Okay, yeah yeah.

A

um But then, if you think it's worth implementing in one case first and then in the other, uh like one one iteration after the next like, we can go for that as well, because I think the environment creation failure is, is the most critical one right.

B

um Yes, yes, uh it should be surfaced. Yes,.

A

Yeah so um let's, let's then uh reconvene these these ideas in the thread. I can comment them over there and then uh you can correct anything, that's necessary, but then, depending on on the feedback that they give, then we move forward with a proposal and split the issues.

A

Does that make sense.

B

Yeah make sense.

A

Thank you cool um one. One last thought that I have is right now, these these errors, they don't surface in the actual job log right.

A

No right is there a way we could have that, because I think it would be nice, oh, like because right now, the job log. As far as I understand it only goes over the the actual scripts there are inside.

B

A

Right but nothing external to them.

B

Yeah yeah, these scripts are just directly reflects what executed in github runner. uh So this is a kind of untouchable place from environment perspective, uh though we can show uh additional information around here. It's uh basically fetching database records uh through the uh build job pipeline, jobs and environments record, and we can show this type of information.

A

Cool yeah. I think it would be nice to show in the log itself um but yeah if it's not possible, then adding an alert on top also works. Well,.

A

Cool um yeah, um I think that's it for me. Do you have anything else.

B

uh No uh yeah, let's keep discussing asynchronously in the issue. uh Yeah.

A

uh Oh and I'll also create an issue for the open live environment button that disappears if the url is invalid. I think that's.

B

A

Issue that we can fix separately, but but it's important to save it so I'll, create an issue for it. Nice all right, thanks for the chat here, thanks all right have a good day cheers yeah, bye.